Hi Vincent, The reason for rejecting the request maybe a number of things like: corrupted request, stale request (for some old session), or for a wrong handle. I don't think we should treat any of these cases as a success. Also, from the MS-SMB2 documentation: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/4f35576a-6f3b-40f0-a832-1c30b0afccb3 "The client MUST locate the file in the GlobalFileTable using the LeaseKey in the Lease Break Notification. If a file is not found, no further processing is required." So I don't think we should be changing the logic here. If SMB v1 had a different behaviour, we should check if that is as per the protocol documentation. If not, change it. Regards, Shyam On Fri, Mar 5, 2021 at 3:12 PM Vincent Whitchurch <vincent.whitchurch@xxxxxxxx> wrote: > > The SMB1 version of ->is_oplock_break() returns true even if the FileId > is not found, as long as the oplock break notification message structure > itself appears to be valid. A true return value makes > cifs_demultiplex_thread() to not print an error message for such > packets. > > However, the SMB2 version returns false in such cases, leading to an > error "No task to wake, unknown frame received!" followed by a hexdump > of the packet header being printed by cifs_demultiplex_thread(). > > Note that before commit fa9c2362497fbd64788063288d ("CIFS: Fix SMB2 > oplock break processing"), SMB2 also returned true for the case where a > connection was found but the FileId was not, but it's not clear to me if > that commit really intended to change the behaviour of the error prints. > > Change the behaviour of SMB2 to be the same as SMB1 and avoid the error > messages for these packets which we ignore as per the spec. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx> > --- > fs/cifs/smb2misc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 60d4bd1eae2b..3ea3bda64083 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -679,7 +679,7 @@ smb2_is_valid_lease_break(char *buffer) > } > spin_unlock(&cifs_tcp_ses_lock); > cifs_dbg(FYI, "Can not process lease break - no lease matched\n"); > - return false; > + return true; > } > > bool > @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > } > spin_unlock(&cifs_tcp_ses_lock); > cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); > - return false; > + return true; > } > > void > -- > 2.28.0 > -- Regards, Shyam
