From: Dan Carpenter <dan.carpenter@linaro.org>
To: Wang Zhaolong <wangzhaolong@huaweicloud.com>
Cc: sfrench@samba.org, pc@manguebit.org, linux-cifs@vger.kernel.org,
samba-technical@lists.samba.org, linux-kernel@vger.kernel.org,
chengzhihao1@huawei.com, yi.zhang@huawei.com,
yangerkun@huawei.com
Subject: Re: [PATCH v4] smb: client: Fix mount deadlock by avoiding super block iteration in DFS reconnect
Date: Tue, 19 Aug 2025 08:25:05 +0300 [thread overview]
Message-ID: <aKQKsS1Vv8joDjo8@stanley.mountain> (raw)
In-Reply-To: <20250815031618.3758759-1-wangzhaolong@huaweicloud.com>
On Fri, Aug 15, 2025 at 11:16:18AM +0800, Wang Zhaolong wrote:
> diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c
> index f65a8a90ba27..37d83aade843 100644
> --- a/fs/smb/client/dfs.c
> +++ b/fs/smb/client/dfs.c
> @@ -429,11 +429,11 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon)
> tcon, tcon->ses->local_nls);
> goto out;
> }
>
> sb = cifs_get_dfs_tcon_super(tcon);
> - if (!IS_ERR(sb))
> + if (!IS_ERR_OR_NULL(sb))
> cifs_sb = CIFS_SB(sb);
>
This is a bad or incomplete fix. When functions return BOTH error
pointers and NULL it MEANS something. The NULL return in this case
is a special kind of success.
For example, if you look up a file, then the an error means the
lookup failed because we're not allowed to have filenames '/' so that's
-EINVAL or maybe there was an allocation failure so that's -ENOMEM or
maybe you don't have access to the directory so it's -EPERM. The NULL
would mean that the lookup succeeded fine, but the file was not found.
Another common use case is "get the LED functions so I can blink
them". -EPROBE_DEFER means the LED subsystem isn't ready yet, but NULL
means the administrator has deliberately disabled it. It's not an error
it's deliberate.
It needs to be documented what the NULL returns *means*. The documentation
is missing here.
See my blog for more details.
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
regards,
dan carpenter
next prev parent reply other threads:[~2025-08-19 5:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-15 3:16 Wang Zhaolong
2025-08-15 17:01 ` Steve French
2025-08-18 14:39 ` Paulo Alcantara
2025-08-19 5:25 ` Dan Carpenter [this message]
2025-08-20 12:08 ` Wang Zhaolong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aKQKsS1Vv8joDjo8@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=chengzhihao1@huawei.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pc@manguebit.org \
--cc=samba-technical@lists.samba.org \
--cc=sfrench@samba.org \
--cc=wangzhaolong@huaweicloud.com \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®