Re: [PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm()

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

 



Yafang Shao <laoar.shao@xxxxxxxxx> writes:

> On Sun, Jun 2, 2024 at 11:52 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>>
>> Yafang Shao <laoar.shao@xxxxxxxxx> writes:
>>
>> > Quoted from Linus [0]:
>> >
>> >   Since user space can randomly change their names anyway, using locking
>> >   was always wrong for readers (for writers it probably does make sense
>> >   to have some lock - although practically speaking nobody cares there
>> >   either, but at least for a writer some kind of race could have
>> >   long-term mixed results
>>
>> Ugh.
>> Ick.
>>
>> This code is buggy.
>>
>> I won't argue that Linus is wrong, about removing the
>> task_lock.
>>
>> Unfortunately strscpy_pad does not work properly with the
>> task_lock removed, and buf_size larger that TASK_COMM_LEN.
>> There is a race that will allow reading past the end
>> of tsk->comm, if we read while tsk->common is being
>> updated.
>
> It appears so. Thanks for pointing it out. Additionally, other code,
> such as the BPF helper bpf_get_current_comm(), also uses strscpy_pad()
> directly without the task_lock. It seems we should change that as
> well.

Which suggests that we could really use a helper that handles all of
the tricky business of reading the tsk->comm lock-free.

Eric





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux