[PATCH] ceph: Move a variable assignment behind a condition check in __ceph_remove_cap()

[PATCH] ceph: Move a variable assignment behind a condition check in __ceph_remove_cap()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Jul 2026 13:21:29 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “__ceph_remove_cap”.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variable “inode” behind a condition check.

This issue was detected by using the Coccinelle software.

Fixes: 38d46409c4639a1d659ebfa70e27a8bed6b8ee1d ("ceph: print cluster fsid and client global_id in all debug logs")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/ceph/caps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 4b37d9ffdf7f..5b6640707949 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -1124,7 +1124,7 @@ void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
 	struct ceph_mds_session *session = cap->session;
 	struct ceph_client *cl = session->s_mdsc->fsc->client;
 	struct ceph_inode_info *ci = cap->ci;
-	struct inode *inode = &ci->netfs.inode;
+	struct inode *inode;
 	struct ceph_mds_client *mdsc;
 	int removed = 0;
 
@@ -1135,7 +1135,7 @@ void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
 	}
 
 	lockdep_assert_held(&ci->i_ceph_lock);
-
+	inode = &ci->netfs.inode;
 	doutc(cl, "%p from %p %llx.%llx\n", cap, inode, ceph_vinop(inode));
 
 	mdsc = ceph_inode_to_fs_client(&ci->netfs.inode)->mdsc;
-- 
2.55.0

Re: [PATCH] ceph: Move a variable assignment behind a condition check in __ceph_remove_cap()

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 13:35 +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 13 Jul 2026 13:21:29 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “__ceph_remove_cap”.
> 
> Thus avoid the risk for undefined behaviour by moving the assignment
> for the variable “inode” behind a condition check.
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 38d46409c4639a1d659ebfa70e27a8bed6b8ee1d ("ceph: print cluster
> fsid and client global_id in all debug logs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  fs/ceph/caps.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 4b37d9ffdf7f..5b6640707949 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -1124,7 +1124,7 @@ void __ceph_remove_cap(struct ceph_cap *cap,
> bool queue_release)
>  	struct ceph_mds_session *session = cap->session;
>  	struct ceph_client *cl = session->s_mdsc->fsc->client;
>  	struct ceph_inode_info *ci = cap->ci;
> -	struct inode *inode = &ci->netfs.inode;
> +	struct inode *inode;
>  	struct ceph_mds_client *mdsc;
>  	int removed = 0;
>  
> @@ -1135,7 +1135,7 @@ void __ceph_remove_cap(struct ceph_cap *cap,
> bool queue_release)
>  	}
>  
>  	lockdep_assert_held(&ci->i_ceph_lock);
> -
> +	inode = &ci->netfs.inode;
>  	doutc(cl, "%p from %p %llx.%llx\n", cap, inode,
> ceph_vinop(inode));
>  
>  	mdsc = ceph_inode_to_fs_client(&ci->netfs.inode)->mdsc;

Makes sense.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Re: [PATCH] ceph: Move a variable assignment behind a condition check in __ceph_remove_cap()

[Dan Carpenter]

On Mon, Jul 13, 2026 at 03:10:40PM -0700, Viacheslav Dubeyko wrote:
> On Mon, 2026-07-13 at 13:35 +0200, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Mon, 13 Jul 2026 13:21:29 +0200
> > 
> > The address of a data structure member was determined before
> > a corresponding null pointer check in the implementation of
> > the function “__ceph_remove_cap”.
> > 
> > Thus avoid the risk for undefined behaviour by moving the assignment
> > for the variable “inode” behind a condition check.
> > 
> > This issue was detected by using the Coccinelle software.
> > 
> > Fixes: 38d46409c4639a1d659ebfa70e27a8bed6b8ee1d ("ceph: print cluster
> > fsid and client global_id in all debug logs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

I have explained to Markus many times that these are not dereferences,
they are just pointer math.  So the original code works fine and does
not need a Fixes tag or a CC to stable.

And then Markus responds, "the C standard says we are not allowed to
dereference NULL pointers"...  Which is true, but again, it's not a
dereference.

regards,
dan carpenter

Re: ceph: Move a variable assignment behind a condition check in __ceph_remove_cap()

[Markus Elfring]

>>> The address of a data structure member was determined before
>>> a corresponding null pointer check in the implementation of
>>> the function “__ceph_remove_cap”.
>>>
>>> Thus avoid the risk for undefined behaviour by moving the assignment
>>> for the variable “inode” behind a condition check.
>>>
>>> This issue was detected by using the Coccinelle software.
>>>
>>> Fixes: 38d46409c4639a1d659ebfa70e27a8bed6b8ee1d ("ceph: print cluster
>>> fsid and client global_id in all debug logs")
…
> I have explained to Markus many times that these are not dereferences,
> they are just pointer math.

How does it help to repeat such a questionable development view?


>                              So the original code works fine and does
> not need a Fixes tag or a CC to stable.
> 
> And then Markus responds, "the C standard says we are not allowed to
> dereference NULL pointers"...  Which is true, but again, it's not a
> dereference.
Why did you get special difficulties with adhering to standard specifications
also in the discussed case?

Regards,
Markus