linux-kernel - Re: [PATCH v2 10/16] overlayfs/file: Convert to cred

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

Message-ID: <87plpukh5c.fsf@intel.com>
Date: Mon, 26 Aug 2024 16:18:07 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Amir Goldstein <amir73il@...il.com>
Cc: brauner@...nel.org, hu1.chen@...el.com, miklos@...redi.hu,
 malini.bhandaru@...el.com, tim.c.chen@...el.com, mikko.ylinen@...el.com,
 lizhen.you@...el.com, linux-unionfs@...r.kernel.org,
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 10/16] overlayfs/file: Convert to cred_guard()

Amir Goldstein <amir73il@...il.com> writes:
>> -       old_cred = ovl_override_creds_light(file_inode(file)->i_sb);
>> -       ret = vfs_fallocate(real.file, mode, offset, len);
>> -       revert_creds_light(old_cred);
>> +       cred_scoped_guard(ovl_creds(file_inode(file)->i_sb))
>> +               ret = vfs_fallocate(real.file, mode, offset, len);
>>
>
> I find this syntax confusing. Even though it is a valid syntax,
> I prefer that if there is a scope we use explicit brackets for it even
> if the scope is
> a single line.
>

Will add the brackets.

> How about using:
>        {
>                cred_guard(ovl_creds(file_inode(file)->i_sb));
>                ret = vfs_fallocate(real.file, mode, offset, len);
>        }
>
> It is more clear and helps averting the compiler bug(?).

I prefer the scoped_cred_guard() idiom, having it spelled out sounds
better to me. But a new block should avoid the bug as well.

>
> Maybe we should just place cred_guard(ovl_creds(file_inode(file_out)->i_sb))
> in ovl_copy_file_range()?
>
> I don't think that the order of ovl_override_creds() vs. inode_lock()
> really matters?
>

Most probably the order should not matter. Will change this.

> Thanks,
> Amir.


Cheers,
-- 
Vinicius