linux-kernel - Re: [PATCH next] smb: client: Fix NULL vs ERR_PTR() returns in cifs_get_tcon

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3059e4f5-8ad7-44f0-80fa-61e3838ed54a@huawei.com>
Date: Tue, 19 Aug 2025 10:48:13 +0800
From: Wang Zhaolong <wangzhaolong1@...wei.com>
To: Steve French <smfrench@...il.com>, Dan Carpenter
	<dan.carpenter@...aro.org>, Paulo Alcantara <pc@...guebit.org>
CC: Ronnie Sahlberg <ronniesahlberg@...il.com>, Shyam Prasad N
	<sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>, Bharath SM
	<bharathsm@...rosoft.com>, <linux-cifs@...r.kernel.org>,
	<samba-technical@...ts.samba.org>, <linux-kernel@...r.kernel.org>,
	<kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH next] smb: client: Fix NULL vs ERR_PTR() returns in
 cifs_get_tcon_super()

> Since Paulo pointed out a problem with v4 of this patch, an obvious
> question is Dan's patch "independent enough" to take or would it make
> the v5 of your patch harder.  Let me know when there is a v5 of the
> patch so we can do more testing and review
> 
> On Mon, Aug 18, 2025 at 8:30 AM Wang Zhaolong
> <wangzhaolong@...weicloud.com> wrote:
>>
Hi Steve, Paulo, and Dan,

Thank you very much for your attention to this patch.

The NULL pointer issue has already been addressed in the V4
version of the patch. I have considered submitting a V5 version
of the patch, but I have not yet decided whether to proceed with it.

While further analyzing the code, I discovered the following
potential issues in the implementation of `cifs_tree_connect()` in
`dfs.c`, which calls `cifs_get_dfs_tcon_super()`:

1. `cifs_get_dfs_tcon_super()` only returns a single superblock reference.
    Using this superblock, it retrieves the associated `cifs_sb`. However,
    a single tcon is supposed to be associated with multiple `cifs_sb`
    instances. I am unsure whether there is a mechanism that guarantees a
    DFS tcon (where `tcon->origin_fullpath` is non-NULL) has exactly one
    associated `cifs_sb`. My current patch series only retrieves the first
    `cifs_sb` that satisfies the condition, which may not be sufficient.

2. The only purpose of retrieving the `cifs_sb` is to update the `prepath`
    field of the `cifs_sb` in `tree_connect_dfs_target()` by calling
    `cifs_update_super_prepath()`. No other information from the VFS-layer
    superblock is used. However, retrieving the superblock is also meant to
    pin it in memory to prevent it from being released. Therefore, I am
    considering whether to simplify the process of updating the prepath.

If I decide to submit a V5 version of the patch, I will need to fully investigate:
1. Whether a DFS tcon can be associated with multiple `cifs_sb` instances.
2. If multiple `cifs_sb` instances are associated with a DFS tcon, whether
    the `prepath` field of all associated `cifs_sb` instances needs to be
     updated.

Only after thoroughly understanding these two points can I come up with a
better implementation for the V5 patch.

Resolving these questions will take some time. If anyone can provide
guidance or assistance, I would greatly appreciate it.

Best regards,
Wang Zhaolong
Wang Zhaolong