[PATCH][SMB3 client] Server interface count incorrectly calculated

[PATCH][SMB3 client] Server interface count incorrectly calculated

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

smb3: interface count displayed incorrectly

The "Server interfaces" count in /proc/fs/cifs/DebugData increases
as the interfaces are requeried, rather than being reset to the new
value.  This could cause a problem if the server disabled
multichannel as the iface_count is checked in try_adding_channels
to see if multichannel still supported.

Cc: <stable@vger.kernel.org>

See attached

-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-interface-count-displayed-incorrectly.patch --]
[-- Type: text/x-patch, Size: 1080 bytes --]

From 1d2aef099583e39610580d20fb86f6f99b64c782 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sat, 15 Oct 2022 17:02:30 -0500
Subject: [PATCH] smb3: interface count displayed incorrectly

The "Server interfaces" count in /proc/fs/cifs/DebugData increases
as the interfaces are requeried, rather than being reset to the new
value.  This could cause a problem if the server disabled
multichannel as the iface_count is checked in try_adding_channels
to see if multichannel still supported.

Cc: <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2ops.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 17b25153cb68..7ee1dd635faf 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -530,6 +530,7 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 	p = buf;
 
 	spin_lock(&ses->iface_lock);
+	ses->iface_count = 0;
 	/*
 	 * Go through iface_list and do kref_put to remove
 	 * any unused ifaces. ifaces in use will be removed
-- 
2.34.1

Re: [PATCH][SMB3 client] Server interface count incorrectly calculated

[Tom Talpey]

On 10/15/2022 6:10 PM, Steve French wrote:
> smb3: interface count displayed incorrectly
> 
> The "Server interfaces" count in /proc/fs/cifs/DebugData increases
> as the interfaces are requeried, rather than being reset to the new
> value.  This could cause a problem if the server disabled
> multichannel as the iface_count is checked in try_adding_channels
> to see if multichannel still supported.
> 
> Cc: <stable@vger.kernel.org>
> 
> See attached
> 

This zeroes the ses->iface_count under the lock, but the whole
routine is dropping and re-taking the same lock many times,
and finally sets the iface_count without holding the lock at
all. Isn't this whole sequence broken?

Tom.

Re: [PATCH][SMB3 client] Server interface count incorrectly calculated

[Tom Talpey]

On 10/17/2022 5:26 PM, Steve French wrote:
> 
> 
> On Mon, Oct 17, 2022, 16:03 Tom Talpey <tom@talpey.com 
> <mailto:tom@talpey.com>> wrote:
> 
>     On 10/15/2022 6:10 PM, Steve French wrote:
>      > smb3: interface count displayed incorrectly
>      >
>      > The "Server interfaces" count in /proc/fs/cifs/DebugData increases
>      > as the interfaces are requeried, rather than being reset to the new
>      > value.  This could cause a problem if the server disabled
>      > multichannel as the iface_count is checked in try_adding_channels
>      > to see if multichannel still supported.
>      >
>      > Cc: <stable@vger.kernel.org <mailto:stable@vger.kernel.org>>
>      >
>      > See attached
>      >
> 
>     This zeroes the ses->iface_count under the lock, but the whole
>     routine is dropping and re-taking the same lock many times,
>     and finally sets the iface_count without holding the lock at
>     all.
> 
> 
> I updated the patch earlier today to fix that existing issue as well 
> (served into same patch). If I missed something let me know

I was just looking at the patch attached to the message I replied to.
I'll look again, but it will have to be tomorrow.

Tom.

Re: [PATCH][SMB3 client] Server interface count incorrectly calculated

[Shyam Prasad N]

On Tue, Oct 18, 2022 at 3:22 AM Steve French <smfrench@gmail.com> wrote:
>
> Updated one is also at
>
> https://git.samba.org/?p=sfrench/cifs-2.6.git;a=patch;h=3f825b8fa93bb300e60c932753e8c5274b253a77
>
> On Mon, Oct 17, 2022, 16:28 Tom Talpey <tom@talpey.com> wrote:
>>
>> On 10/17/2022 5:26 PM, Steve French wrote:
>> >
>> >
>> > On Mon, Oct 17, 2022, 16:03 Tom Talpey <tom@talpey.com
>> > <mailto:tom@talpey.com>> wrote:
>> >
>> >     On 10/15/2022 6:10 PM, Steve French wrote:
>> >      > smb3: interface count displayed incorrectly
>> >      >
>> >      > The "Server interfaces" count in /proc/fs/cifs/DebugData increases
>> >      > as the interfaces are requeried, rather than being reset to the new
>> >      > value.  This could cause a problem if the server disabled
>> >      > multichannel as the iface_count is checked in try_adding_channels
>> >      > to see if multichannel still supported.
>> >      >
>> >      > Cc: <stable@vger.kernel.org <mailto:stable@vger.kernel.org>>
>> >      >
>> >      > See attached
>> >      >
>> >
>> >     This zeroes the ses->iface_count under the lock, but the whole
>> >     routine is dropping and re-taking the same lock many times,
>> >     and finally sets the iface_count without holding the lock at
>> >     all.
>> >
>> >
>> > I updated the patch earlier today to fix that existing issue as well
>> > (served into same patch). If I missed something let me know
>>
>> I was just looking at the patch attached to the message I replied to.
>> I'll look again, but it will have to be tomorrow.
>>
>> Tom.

Looks good to me for fixing the issue.
One or two more changes needed in this area. You can expect additional
patches from me in the coming days.

-- 
Regards,
Shyam