linux-kernel - Re: [PATCH] cifs: Fix TCP_Server

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH2r5mvOwmdcP_5kjC+ENgtooi06AuPvwBXrMnZrfy7_poAoFQ@mail.gmail.com>
Date: Mon, 20 Oct 2025 11:58:42 -0500
From: Steve French <smfrench@...il.com>
To: David Howells <dhowells@...hat.com>
Cc: Enzo Matsumiya <ematsumiya@...e.de>, Steve French <sfrench@...ba.org>, 
	Paulo Alcantara <pc@...guebit.org>, linux-cifs@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Pavel Shilovsky <piastryyy@...il.com>, ronnie sahlberg <ronniesahlberg@...il.com>, 
	Bharath S M <bharathsm@...rosoft.com>, Shyam Prasad <nspmangalore@...il.com>
Subject: Re: [PATCH] cifs: Fix TCP_Server_Info::credits to be signed

On Mon, Oct 20, 2025 at 9:08 AM David Howells <dhowells@...hat.com> wrote:
>
> Enzo Matsumiya <ematsumiya@...e.de> wrote:
>
> > Both semantically and technically, credits shouldn't go negative.
> > Shouldn't those other fields/functions become unsigned instead?
>
> That's really a question for Steve, but it makes it easier to handle
> underflow, and I'm guessing that the maximum credits isn't likely to exceed
> 2G.
>
> David

Interesting question - I do like the idea of keeping signed if it
makes it easier to check
for underflows but IIRC that hasn't been a problem in a long time (adding Pavel
and Ronnie in case they remember) but more important than the signed
vs. unsigned
in my opinion is at least keeping the field consistent.

I have seen a few stress related xfstests that often generate
reconnects, so we may want
to run with the tracepoint enabled
(smb3_reconnect_with_invalid_credits) to see if that
is actually happening (the underflow of credits)

I also was thinking that we should doublecheck that lease break acks
will never run out credits
(since that can deadlock servers for more than 30 seconds as they wait
for timeouts), even if
"lease break storms" are rare.   Maybe we should allow e.g. lease
break acks to borrow echo
credits e.g. as minor improvement?

-- 
Thanks,

Steve