linux-kernel - Re: [PATCH v2 2/2] fs: add the ability for statmount() to report the mount devicename

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7554bd2b13330f8b44b23b775dd4d611ad6cdcae.camel@kernel.org>
Date: Thu, 07 Nov 2024 06:15:40 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Jan Kara <jack@...e.cz>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
	 <brauner@...nel.org>, Josef Bacik <josef@...icpanda.com>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] fs: add the ability for statmount() to report
 the mount devicename

On Thu, 2024-11-07 at 10:40 +0100, Jan Kara wrote:
> On Wed 06-11-24 14:53:06, Jeff Layton wrote:
> > /proc/self/mountinfo displays the devicename for the mount, but
> > statmount() doesn't yet have a way to return it. Add a new
> > STATMOUNT_MNT_DEVNAME flag, claim the 32-bit __spare1 field to hold the
> > offset into the str[] array. STATMOUNT_MNT_DEVNAME will only be set in
> > the return mask if there is a device string.
> > 
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> 
> Just one question below:
> 
> > @@ -5078,6 +5091,12 @@ static int statmount_string(struct kstatmount *s, u64 flag)
> >  		if (seq->count == sm->fs_subtype)
> >  			return 0;
> >  		break;
> > +	case STATMOUNT_MNT_DEVNAME:
> > +		sm->mnt_devname = seq->count;
> > +		ret = statmount_mnt_devname(s, seq);
> > +		if (seq->count == sm->mnt_devname)
> 
> Why this odd check? Why don't you rather do:
> 		if (ret)
> ?
>

statmount_mnt_devname() can return without emitting anything to the seq
if ->show_devname and r->mnt_devname are both NULL. In that case, we
don't want statmount_string() to return an error, but we also don't
want to do any further manipulation of the seq. So, the above handles
either the case where show_devname returned an error and the case where
there was nothing to emit.

I did consider having statmount_mnt_devname() return -ENOBUFS if there
was nothing to emit, and then handle that in the caller, but checking
to see whether the seq has advanced seemed like a cleaner solution.

I can add a comment to that effect since it does look a bit confusing.


> > +			return ret;
> > +		break;
> >  	default:
> >  		WARN_ON_ONCE(true);
> >  		return -EINVAL;
> 
> 								Honza
> 

-- 
Jeff Layton <jlayton@...nel.org>