Re: [PATCH] inotify: invalid mask should return a error number but not set it

[Zhao Hongjiang <zhaohongjiang@huawei.com>]

On 2013/4/19 6:19, Paul Gortmaker wrote:
> On 13-04-18 12:49 AM, Zhao Hongjiang wrote:
>> When we run the crackerjack testsuit, inotify_add_watch test is stalled cause the
>> invalid mask 0, the task is waiting for the event but it never come. This should
>> return -EINVAL and it do is before the commit 676a0675cf9200 ("inotify: remove 
>> broken mask checks causing unmount to be EINVAL"). The commit remove the invalid 
>> mask check simply, but the invalid mask check is needed indeed.
>>
>> Check the mask wether in the ALL_INOTIFY_BITS before the inotify_arg_to_mask call,
>> if is not, just return -EINVAL. 
> 
> If you want to filter out zero as EINVAL, then it should be done
> right at the top of the syscall entry  and not duplicated twice
> here in the guts of the code, I think.  I can send a suggested
> patch if that description is not 100% self evident.
> 
Not only filter out zero but also other invalid mask like 0x00001000 which are not in
ALL_INOTIFY_BITS. i just leave the check at the place before, but move to the top of
the syscall entry is more reasonable. I'll send the v2 shortly. Thanks.

Zhao Hongjiang.
> Paul.
> --
> 
>>
>> Because IN_UNMOUNT is in ALL_INOTIFY_BITS, so this change will not trigger the 
>> problem that above commit fixed.
>>
>> Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  fs/notify/inotify/inotify_user.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
>> index e0f7c12..b024bc1 100644
>> --- a/fs/notify/inotify/inotify_user.c
>> +++ b/fs/notify/inotify/inotify_user.c
>> @@ -573,6 +573,9 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
>>  	int ret;
>>  
>>  	/* don't allow invalid bits: we don't want flags set */
>> +	if (unlikely(!(arg & ALL_INOTIFY_BITS)))
>> +		return -EINVAL;
>> +
>>  	mask = inotify_arg_to_mask(arg);
>>  
>>  	fsn_mark = fsnotify_find_inode_mark(group, inode);
>> @@ -624,6 +627,9 @@ static int inotify_new_watch(struct fsnotify_group *group,
>>  	spinlock_t *idr_lock = &group->inotify_data.idr_lock;
>>  
>>  	/* don't allow invalid bits: we don't want flags set */
>> +	if (unlikely(!(arg & ALL_INOTIFY_BITS)))
>> +		return -EINVAL;
>> +
>>  	mask = inotify_arg_to_mask(arg);
>>  
>>  	tmp_i_mark = kmem_cache_alloc(inotify_inode_mark_cachep, GFP_KERNEL);
>> -- 1.7.1
>>
> 
> .
>