Re: [PATCH] Smack: Provide read control for io_uring_cmd

[Casey Schaufler <casey@schaufler-ca.com>]

On 8/27/2022 8:59 AM, Kanchan Joshi wrote:
> On Tue, Aug 23, 2022 at 04:46:18PM -0700, Casey Schaufler wrote:
>> Limit io_uring "cmd" options to files for which the caller has
>> Smack read access. There may be cases where the cmd option may
>> be closer to a write access than a read, but there is no way
>> to make that determination.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> -- 
>> security/smack/smack_lsm.c | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 001831458fa2..bffccdc494cb 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -42,6 +42,7 @@
>> #include <linux/fs_context.h>
>> #include <linux/fs_parser.h>
>> #include <linux/watch_queue.h>
>> +#include <linux/io_uring.h>
>> #include "smack.h"
>>
>> #define TRANS_TRUE    "TRUE"
>> @@ -4732,6 +4733,36 @@ static int smack_uring_sqpoll(void)
>>     return -EPERM;
>> }
>>
>> +/**
>> + * smack_uring_cmd - check on file operations for io_uring
>> + * @ioucmd: the command in question
>> + *
>> + * Make a best guess about whether a io_uring "command" should
>> + * be allowed. Use the same logic used for determining if the
>> + * file could be opened for read in the absence of better criteria.
>> + */
>> +static int smack_uring_cmd(struct io_uring_cmd *ioucmd)
>> +{
>> +    struct file *file = ioucmd->file;
>> +    struct smk_audit_info ad;
>> +    struct task_smack *tsp;
>> +    struct inode *inode;
>> +    int rc;
>> +
>> +    if (!file)
>> +        return -EINVAL;
>> +
>> +    tsp = smack_cred(file->f_cred);
>> +    inode = file_inode(file);
>> +
>> +    smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH);
>> +    smk_ad_setfield_u_fs_path(&ad, file->f_path);
>> +    rc = smk_tskacc(tsp, smk_of_inode(inode), MAY_READ, &ad);
>> +    rc = smk_bu_credfile(file->f_cred, file, MAY_READ, rc);
>> +
>> +    return rc;
>> +}
>> +
>> #endif /* CONFIG_IO_URING */
>>
>> struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
>> @@ -4889,6 +4920,7 @@ static struct security_hook_list smack_hooks[]
>> __lsm_ro_after_init = {
>> #ifdef CONFIG_IO_URING
>>     LSM_HOOK_INIT(uring_override_creds, smack_uring_override_creds),
>>     LSM_HOOK_INIT(uring_sqpoll, smack_uring_sqpoll),
>> +    LSM_HOOK_INIT(uring_cmd, smack_uring_cmd),
>> #endif
>
> Tried this on nvme device (/dev/ng0n1).
> Took a while to come out of noob setup-related issues but I see that
> smack is listed (in /sys/kernel/security/lsm), smackfs is present, and
> the hook (smack_uring_cmd) gets triggered fine on doing I/O on
> /dev/ng0n1.
>
> I/O goes fine, which seems aligned with the label on /dev/ng0n1 (which
> is set to floor).
>
> $ chsmack -L /dev/ng0n1
> /dev/ng0n1 access="_"

Setting the Smack on the object that the cmd operates on to
something other than "_" would be the correct test. If that
is /dev/ng0n1 you could use

	# chsmack -a Snap /dev/ng0n1

The unprivileged user won't be able to read /dev/ng0n1 so you
won't get as far as testing the cmd interface. I don't know
io_uring and nvme well enough to know what other objects may
be involved. Noob here, too.

>
> I ran fio (/usr/bin/fio), which also has the same label.
> Hope you expect the same outcome.
>
> Do you run something else to see that things are fine e.g. for
> /dev/null, which also has the same label "_".
> If yes, I can try the same on nvme side.
>