Re: [PATCH] fs: clear file set[ug]id when writing via mmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jan Kara <jack@xxxxxxx> writes:

> On Thu 19-11-15 16:10:43, Kees Cook wrote:
>> Normally, when a user can modify a file that has setuid or setgid bits,
>> those bits are cleared when they are not the file owner or a member of the
>> group. This is enforced when using write() directly but not when writing
>> to a shared mmap on the file. This could allow the file writer to gain
>> privileges by changing the binary without losing the setuid/setgid bits.
>> 
>> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>
> So I had another look at this and now I understand why we didn't do it from
> the start:
>
> To call file_remove_privs() safely, we need to hold inode->i_mutex since
> that operations is going to modify file mode / extended attributes and
> i_mutex protects those. However we cannot get i_mutex in the page fault
> path as that ranks above mmap_sem which we hold during the whole page
> fault.
>
> So calling file_remove_privs() when opening the file is probably as good as
> it can get. It doesn't catch the case when suid bits / IMA attrs are set
> while the file is already open but I don't see easy way around this.

Could we perhaps do this on mmap MAP_WRITE instead of open, and simply
deny adding these attributes if a file is mapped for write?

That would seem to be a little more compatible with what we already do,
and guards against the races you mention as well.

Eric

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]