linux-kernel - Re: [PATCH review 05/16] xfs: Update xfs_ioctl_setattr to handle projids in any user namespace

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: <51F616F2.5040906@cn.fujitsu.com>
Date:	Mon, 29 Jul 2013 15:17:06 +0800
From:	Gao feng <gaofeng@...fujitsu.com>
To:	Dave Chinner <david@...morbit.com>
CC:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-fsdevel@...r.kernel.org,
	Linux Containers <containers@...ts.linux-foundation.org>,
	linux-kernel@...r.kernel.org, "Serge E. Hallyn" <serge@...lyn.com>,
	Ben Myers <bpm@....com>, Alex Elder <elder@...nel.org>
Subject: Re: [PATCH review 05/16] xfs: Update xfs_ioctl_setattr to handle
 projids in any user namespace

On 02/19/2013 09:55 AM, Dave Chinner wrote:
> On Sun, Feb 17, 2013 at 05:10:58PM -0800, Eric W. Biederman wrote:
>> From: "Eric W. Biederman" <ebiederm@...ssion.com>
>>
>> - Convert the userspace value in fa->fsx_projid into a kprojid and
>>   store it in the variable projid.
>> - Verify that xfs can store the projid after it is converted into
>>   xfs's user namespace.
>> - Replace uses of fa->fsx_projid with projid throughout
>>   xfs_ioctl_setattr.
>>
>> Cc: Ben Myers <bpm@....com>
>> Cc: Alex Elder <elder@...nel.org>
>> Cc: Dave Chinner <david@...morbit.com>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>> ---
>>  fs/xfs/xfs_ioctl.c |   26 ++++++++++++++++++--------
>>  1 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>> index 016624b..4a55f50 100644
>> --- a/fs/xfs/xfs_ioctl.c
>> +++ b/fs/xfs/xfs_ioctl.c
>> @@ -925,6 +925,7 @@ xfs_ioctl_setattr(
>>  	struct xfs_dquot	*gdqp = NULL;
>>  	struct xfs_dquot	*olddquot = NULL;
>>  	int			code;
>> +	kprojid_t		projid = INVALID_PROJID;
>>  
>>  	trace_xfs_ioctl_setattr(ip);
>>  
>> @@ -934,11 +935,20 @@ xfs_ioctl_setattr(
>>  		return XFS_ERROR(EIO);
>>  
>>  	/*
>> -	 * Disallow 32bit project ids when projid32bit feature is not enabled.
>> +	 * Verify the specifid project id is valid.
>>  	 */
>> -	if ((mask & FSX_PROJID) && (fa->fsx_projid > (__uint16_t)-1) &&
>> -			!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb))
>> -		return XFS_ERROR(EINVAL);
>> +	if (mask & FSX_PROJID) {
>> +		projid = make_kprojid(current_user_ns(), fa->fsx_projid);
>> +		if (!projid_valid(projid))
>> +			return XFS_ERROR(EINVAL);
>> +
>> +		/*
>> +		 * Disallow 32bit project ids when projid32bit feature is not enabled.
>> +		 */
>> +		if ((from_kprojid(&init_user_ns, projid) > (__uint16_t)-1) &&
>> +		    !xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb))
>> +			return XFS_ERROR(EINVAL);
>> +	}
> 
> That looks busted. Why does one use current_user_ns() and the other
> &init_user_ns()?
> 

hmm, through this thread had been stopped discussing for a long time, but I'm working on converting
ids to kids for xfs now, and I want to remove the finial dependenciy for user namespace.

Maybe I can explain why.

projid = make_kprojid(current_user_ns(), fa->fsx_projid);

this code translates the projid in current user namespace to the global kernel projid(kprojid_t).

and from_kprojid(&init_user_ns, projid) here is equal to projid.val, since the real value
of global kernel projid and projid is same in init user namespace.

After translating projid to global kernel projid, we should check if the real value of this
global kernel projid is valid.

I think maybe just modifying from_kprojid(&init_user_ns, projid) to projid.val here is easier
to understand.


Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/