Re: [PATCH] inode_has_no_xattr() does not use proper sync

Alexander Lochmann
2018-12-05T11:43:39Z

[-- Attachment #1.1: Type: text/plain, Size: 2388 bytes --]

Am 05.12.18 um 10:01 schrieb Jan Kara:
> On Tue 27-11-18 15:54:28, Alexander Lochmann wrote:
>>
>> inode.i_flags is modified without any proper
>> synchronisation used. inode_set_flags() is now used.
>>
>> Found by LockDoc (Alexander Lochmann, Horst Schirmeier and Olaf
>> Spinczyk)
>>
>> Signed-off-by: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
>> Signed-off-by: Horst Schirmeier <horst.schirmeier@tu-dortmund.de>
>
> Thanks for the patch! Couple notes to this patch:
>
> 1) This is a generic VFS helper as such, linux-fsdevel mailing list and VFS
> maintainer Al Viro is the right forum to post this patch to. We do have
> scripts/get_maintainer.pl script you can use on a patch / file to get idea
> who's the best to post the change to. It is not perfect but usually works
> fine.
Oh, that's my fault. I thought this ml was the right place.
>
> 2) It would be good to include stacktrace showing where the unlocked access
> happens in the changelog. It is non-trivial to find it by brief inspection
> as all standard filesystems call inode_has_no_xattr() under i_rwsem. This
> problem is really specific to blkdev_write_iter() AFAICT.
>
> 3) Also can you please add comment into inode_has_no_xattr() like:
> /*
> * blkdev_write_iter() can call this without i_rwsem, need to be
> * careful with i_flags update.
> */
2) + 3) Done. Will post the patch asap.

- Alex
>
> Honza
>> ---
>> include/linux/fs.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index c95c0807471f..54f3a21668a6 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -3446,7 +3446,7 @@ static inline int check_sticky(struct inode *dir,
>> struct inode *inode)
>> static inline void inode_has_no_xattr(struct inode *inode)
>> {
>> if (!is_sxid(inode->i_mode) && (inode->i_sb->s_flags & SB_NOSEC))
>> - inode->i_flags |= S_NOSEC;
>> + inode_set_flags(inode, S_NOSEC, S_NOSEC);
>> }
>>
>> static inline bool is_root_inode(struct inode *inode)
>> --
>> 2.19.1
>>
>
>
>

--
Technische Universität Dortmund
Alexander Lochmann PGP key: 0xBC3EF6FD
Otto-Hahn-Str. 16 phone: +49.231.7556141
D-44227 Dortmund fax: +49.231.7556116
http://ess.cs.tu-dortmund.de/Staff/al

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]