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

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] [day] [month] [year] [list]

Message-ID: <8921ec9e-7c4c-71e2-0397-9e564824d9a4@tu-dortmund.de>
Date:   Wed, 5 Dec 2018 12:43:18 +0100
From:   Alexander Lochmann <alexander.lochmann@...dortmund.de>
To:     Jan Kara <jack@...e.cz>
Cc:     Horst Schirmeier <horst.schirmeier@...dortmund.de>,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH] inode_has_no_xattr() does not use proper sync



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@...dortmund.de>
>> Signed-off-by: Horst Schirmeier <horst.schirmeier@...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



Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)