NAK: [SRU][J][PATCH 2/2] smb: client: fix potential UAF in smb2_is_valid_lease_break()

[Stewart Hore]

On Thu, Feb 06, 2025 at 08:10:53PM -0500, Yuxuan Luo wrote:
> From: Paulo Alcantara <pc at manguebit.com>
>
> Skip sessions that are being teared down (status == SES_EXITING) to
> avoid UAF.
>
> Cc: stable at vger.kernel.org
> Signed-off-by: Paulo Alcantara (Red Hat) <pc at manguebit.com>
> Signed-off-by: Steve French <stfrench at microsoft.com>
> (backported from commit 705c76fbf726c7a2f6ff9143d4013b18daaaebf1)
> [yuxuan.luo: ignored context conflicts and added the if statement.]
> CVE-2024-35864
> Signed-off-by: Yuxuan Luo <yuxuan.luo at canonical.com>
> ---
>  fs/cifs/smb2misc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 8f409404aee1..c28241dcc950 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -604,6 +604,8 @@ smb2_is_valid_lease_break(char *buffer)
>  	/* look up tcon based on tid & uid */
>  	spin_lock(&cifs_tcp_ses_lock);
>  	list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
> +		if (cifs_ses_exiting(ses))
> +			continue;
>  		list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
>  			list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
>  				spin_lock(&tcon->open_file_lock);
> --
> 2.43.0

The call to `cifs_ses_exiting(ses)` will dereference the `ses` pointer
before it has been assigned.

Suggested change, move the `if(cifs_ses_exiting(ses))` to after `ses`
assignment. E.g.:

```
        list_for_each(tmp, &server->smb_ses_list) {
  		      list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+               if (cifs_ses_exiting(ses))
+                   continue;
  			        list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
```

--
kernel-team mailing list
kernel-team at lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team