| Message ID | 83a121d5-a2ec-197b-708c-9ea2f9d0bd6a@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Smack: Provide read control for io_uring_cmd | expand |
On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 ... > @@ -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; Perhaps this is a better question for Jens, but ioucmd->file is always going to be valid when the LSM hook is called, yes? > + 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 */
On 8/23/22 6:05 PM, Paul Moore wrote: > On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 > > ... > >> @@ -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; > > Perhaps this is a better question for Jens, but ioucmd->file is always > going to be valid when the LSM hook is called, yes? file will always be valid for uring commands, as they are marked as requiring a file. If no valid fd is given for it, it would've been errored early on, before reaching f_op->uring_cmd().
On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: > On 8/23/22 6:05 PM, Paul Moore wrote: > > On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 > > > > ... > > > >> @@ -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; > > > > Perhaps this is a better question for Jens, but ioucmd->file is always > > going to be valid when the LSM hook is called, yes? > > file will always be valid for uring commands, as they are marked as > requiring a file. If no valid fd is given for it, it would've been > errored early on, before reaching f_op->uring_cmd(). Hey Casey, where do things stand with this patch? To be specific, did you want me to include this in the lsm/stable-6.0 PR for Linus or are you planning to send it separately? If you want me to send it up, are you planning another revision? There is no right or wrong answer here as far as I'm concerned, I'm just trying to make sure we are all on the same page.
On 8/26/2022 8:15 AM, Paul Moore wrote: > On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: >> On 8/23/22 6:05 PM, Paul Moore wrote: >>> On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 >>> ... >>> >>>> @@ -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; >>> Perhaps this is a better question for Jens, but ioucmd->file is always >>> going to be valid when the LSM hook is called, yes? >> file will always be valid for uring commands, as they are marked as >> requiring a file. If no valid fd is given for it, it would've been >> errored early on, before reaching f_op->uring_cmd(). > Hey Casey, where do things stand with this patch? To be specific, did > you want me to include this in the lsm/stable-6.0 PR for Linus or are > you planning to send it separately? If you want me to send it up, are > you planning another revision? > > There is no right or wrong answer here as far as I'm concerned, I'm > just trying to make sure we are all on the same page. I think the whole LSM fix for io_uring looks better the more complete it is. I don't see the Smack check changing until such time as there's better information available to make decisions upon. If you send it along with the rest of the patch set I think we'll have done our best.
On Fri, Aug 26, 2022 at 12:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 8/26/2022 8:15 AM, Paul Moore wrote: > > On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: > >> On 8/23/22 6:05 PM, Paul Moore wrote: > >>> On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 > >>> ... > >>> > >>>> @@ -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; > >>> Perhaps this is a better question for Jens, but ioucmd->file is always > >>> going to be valid when the LSM hook is called, yes? > >> file will always be valid for uring commands, as they are marked as > >> requiring a file. If no valid fd is given for it, it would've been > >> errored early on, before reaching f_op->uring_cmd(). > > Hey Casey, where do things stand with this patch? To be specific, did > > you want me to include this in the lsm/stable-6.0 PR for Linus or are > > you planning to send it separately? If you want me to send it up, are > > you planning another revision? > > > > There is no right or wrong answer here as far as I'm concerned, I'm > > just trying to make sure we are all on the same page. > > I think the whole LSM fix for io_uring looks better the more complete > it is. I don't see the Smack check changing until such time as there's > better information available to make decisions upon. If you send it along > with the rest of the patch set I think we'll have done our best. Okay, will do. Would you like me to tag the patch with the 'Fixes:' and stable tags, similar to the LSM and SELinux patches?
On 8/26/2022 11:59 AM, Paul Moore wrote: > On Fri, Aug 26, 2022 at 12:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 8/26/2022 8:15 AM, Paul Moore wrote: >>> On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: >>>> On 8/23/22 6:05 PM, Paul Moore wrote: >>>>> On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 >>>>> ... >>>>> >>>>>> @@ -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; >>>>> Perhaps this is a better question for Jens, but ioucmd->file is always >>>>> going to be valid when the LSM hook is called, yes? >>>> file will always be valid for uring commands, as they are marked as >>>> requiring a file. If no valid fd is given for it, it would've been >>>> errored early on, before reaching f_op->uring_cmd(). >>> Hey Casey, where do things stand with this patch? To be specific, did >>> you want me to include this in the lsm/stable-6.0 PR for Linus or are >>> you planning to send it separately? If you want me to send it up, are >>> you planning another revision? >>> >>> There is no right or wrong answer here as far as I'm concerned, I'm >>> just trying to make sure we are all on the same page. >> I think the whole LSM fix for io_uring looks better the more complete >> it is. I don't see the Smack check changing until such time as there's >> better information available to make decisions upon. If you send it along >> with the rest of the patch set I think we'll have done our best. > Okay, will do. Would you like me to tag the patch with the 'Fixes:' > and stable tags, similar to the LSM and SELinux patches? Yes, I think that's best.
On Fri, Aug 26, 2022 at 3:04 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 8/26/2022 11:59 AM, Paul Moore wrote: > > On Fri, Aug 26, 2022 at 12:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> On 8/26/2022 8:15 AM, Paul Moore wrote: > >>> On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: > >>>> On 8/23/22 6:05 PM, Paul Moore wrote: > >>>>> On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 > >>>>> ... > >>>>> > >>>>>> @@ -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; > >>>>> Perhaps this is a better question for Jens, but ioucmd->file is always > >>>>> going to be valid when the LSM hook is called, yes? > >>>> file will always be valid for uring commands, as they are marked as > >>>> requiring a file. If no valid fd is given for it, it would've been > >>>> errored early on, before reaching f_op->uring_cmd(). > >>> Hey Casey, where do things stand with this patch? To be specific, did > >>> you want me to include this in the lsm/stable-6.0 PR for Linus or are > >>> you planning to send it separately? If you want me to send it up, are > >>> you planning another revision? > >>> > >>> There is no right or wrong answer here as far as I'm concerned, I'm > >>> just trying to make sure we are all on the same page. > >> I think the whole LSM fix for io_uring looks better the more complete > >> it is. I don't see the Smack check changing until such time as there's > >> better information available to make decisions upon. If you send it along > >> with the rest of the patch set I think we'll have done our best. > > Okay, will do. Would you like me to tag the patch with the 'Fixes:' > > and stable tags, similar to the LSM and SELinux patches? > > Yes, I think that's best. Done and merged to lsm/stable-6.0. I'm going to let the automated stuff do it's thing and assuming no problems I'll plan to send it to Linus on Monday ... sending stuff like this last thing on a Friday is a little too risky for my tastes.
On 8/26/2022 12:10 PM, Paul Moore wrote: > On Fri, Aug 26, 2022 at 3:04 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 8/26/2022 11:59 AM, Paul Moore wrote: >>> On Fri, Aug 26, 2022 at 12:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> On 8/26/2022 8:15 AM, Paul Moore wrote: >>>>> On Tue, Aug 23, 2022 at 8:07 PM Jens Axboe <axboe@kernel.dk> wrote: >>>>>> On 8/23/22 6:05 PM, Paul Moore wrote: >>>>>>> On Tue, Aug 23, 2022 at 7:46 PM Casey Schaufler <casey@schaufler-ca.com> 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 >>>>>>> ... >>>>>>> >>>>>>>> @@ -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; >>>>>>> Perhaps this is a better question for Jens, but ioucmd->file is always >>>>>>> going to be valid when the LSM hook is called, yes? >>>>>> file will always be valid for uring commands, as they are marked as >>>>>> requiring a file. If no valid fd is given for it, it would've been >>>>>> errored early on, before reaching f_op->uring_cmd(). >>>>> Hey Casey, where do things stand with this patch? To be specific, did >>>>> you want me to include this in the lsm/stable-6.0 PR for Linus or are >>>>> you planning to send it separately? If you want me to send it up, are >>>>> you planning another revision? >>>>> >>>>> There is no right or wrong answer here as far as I'm concerned, I'm >>>>> just trying to make sure we are all on the same page. >>>> I think the whole LSM fix for io_uring looks better the more complete >>>> it is. I don't see the Smack check changing until such time as there's >>>> better information available to make decisions upon. If you send it along >>>> with the rest of the patch set I think we'll have done our best. >>> Okay, will do. Would you like me to tag the patch with the 'Fixes:' >>> and stable tags, similar to the LSM and SELinux patches? >> Yes, I think that's best. > Done and merged to lsm/stable-6.0. I'm going to let the automated > stuff do it's thing and assuming no problems I'll plan to send it to > Linus on Monday ... sending stuff like this last thing on a Friday is > a little too risky for my tastes. Agreed. Thank you.
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="_" 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.
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. >
Hey Casey I have tested this patch and I see the smack_uring_cmd prevents access to file when smack labels are different. They way I got there was a bit convoluted and I'll describe it here so ppl can judge how valid the test is. Tested-by: Joel Granados <j.granados@samsung.com> I started by reproducing what Kanchan had done by changing the smack label from "_" to "Snap". Then I ran the io_uring passthrough test included in liburing with an unprivileged user and saw that the smack_uring_cmd function was NOT executed because smack prevented an open on the device file ("/dev/ng0n1" on my box). So here I got a bit sneaky and changed the label after the open to the device was done. This is how I did it: 1. In the io_uring_passthrough.c tests I stopped execution after the device was open and before the actual IO. 2. While on hold I changed the label of the device from "_" to "Snap". At this point the device had a "Snap" label and the executable had a "_" label. 3. Previous to execution I had placed a printk inside the smack_uring_cmd function. And I also made sure to printk whenever security_uring_command returned because of a security violation. 4. I continued execution and saw that both smack_uiring_cmd and io_uring_cmd returned error. Which is what I expected. I'm still a bit unsure of my setup so if anyone has comments or a way to make it better, I would really appreciate feedback. Best Joel On Mon, Aug 29, 2022 at 09:20:09AM -0700, Casey Schaufler wrote: > 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. > >
On 8/30/2022 6:08 AM, Joel Granados wrote: > Hey Casey > > I have tested this patch and I see the smack_uring_cmd prevents access > to file when smack labels are different. They way I got there was a bit > convoluted and I'll describe it here so ppl can judge how valid the test > is. > > Tested-by: Joel Granados <j.granados@samsung.com> Thank you. > > I started by reproducing what Kanchan had done by changing the smack > label from "_" to "Snap". Then I ran the io_uring passthrough test > included in liburing with an unprivileged user and saw that the > smack_uring_cmd function was NOT executed because smack prevented an open on > the device file ("/dev/ng0n1" on my box). > > So here I got a bit sneaky and changed the label after the open to the > device was done. This is how I did it: > 1. In the io_uring_passthrough.c tests I stopped execution after the > device was open and before the actual IO. > 2. While on hold I changed the label of the device from "_" to "Snap". > At this point the device had a "Snap" label and the executable had a > "_" label. > 3. Previous to execution I had placed a printk inside the > smack_uring_cmd function. And I also made sure to printk whenever > security_uring_command returned because of a security violation. > 4. I continued execution and saw that both smack_uiring_cmd and > io_uring_cmd returned error. Which is what I expected. > > I'm still a bit unsure of my setup so if anyone has comments or a way to > make it better, I would really appreciate feedback. This is a perfectly rational approach to the test. Another approach would be to add a Smack access rule: echo "_ Snap r" > /sys/fs/smackfs/load2 and label the device before the test begins. Step 2 changes from labeling the device to removing the access rule: echo "_ Snap -" > /sys/fs/smackfs/load2 and you will get the same result. I wouldn't change your test, but I would probably add another that does it using the rule change. > Best > > Joel > > On Mon, Aug 29, 2022 at 09:20:09AM -0700, Casey Schaufler wrote: >> 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. >>>
On Tue, Aug 30, 2022 at 07:16:55AM -0700, Casey Schaufler wrote: > On 8/30/2022 6:08 AM, Joel Granados wrote: > > Hey Casey > > > > I have tested this patch and I see the smack_uring_cmd prevents access > > to file when smack labels are different. They way I got there was a bit > > convoluted and I'll describe it here so ppl can judge how valid the test > > is. > > > > Tested-by: Joel Granados <j.granados@samsung.com> > > Thank you. np > > > > > I started by reproducing what Kanchan had done by changing the smack > > label from "_" to "Snap". Then I ran the io_uring passthrough test > > included in liburing with an unprivileged user and saw that the > > smack_uring_cmd function was NOT executed because smack prevented an open on > > the device file ("/dev/ng0n1" on my box). > > > > So here I got a bit sneaky and changed the label after the open to the > > device was done. This is how I did it: > > 1. In the io_uring_passthrough.c tests I stopped execution after the > > device was open and before the actual IO. > > 2. While on hold I changed the label of the device from "_" to "Snap". > > At this point the device had a "Snap" label and the executable had a > > "_" label. > > 3. Previous to execution I had placed a printk inside the > > smack_uring_cmd function. And I also made sure to printk whenever > > security_uring_command returned because of a security violation. > > 4. I continued execution and saw that both smack_uiring_cmd and > > io_uring_cmd returned error. Which is what I expected. > > > > I'm still a bit unsure of my setup so if anyone has comments or a way to > > make it better, I would really appreciate feedback. > > This is a perfectly rational approach to the test. Another approach > would be to add a Smack access rule: > > echo "_ Snap r" > /sys/fs/smackfs/load2 > > and label the device before the test begins. Step 2 changes from labeling > the device to removing the access rule: > > echo "_ Snap -" > /sys/fs/smackfs/load2 > > and you will get the same result. I wouldn't change your test, but I > would probably add another that does it using the rule change. Followed your proposal and I could see that it went passed the "file open: permission denied" error. However it did not execute smack_uring_cmd as smack prevented execution of an ioctl call [1]. This is probably because the test that I'm using from liburing does a lot of things to set things up besides just opening the device. I tried several strings on /sys/fs/smackfs/load2 but had no luck at actually arriving to the smack_uring_cmd function. Here is what I tried: 1. echo "_ Snap r-x---" > /sys/fs/smackfs/load2 which prevented access but not in smack_uring_cmd 2. echo "_ Snap -wx---" > /sys/fs/smackfs/load2 This of course prevented me from opening the /dev/ng0n1 3. echo "_ Snap rw----" > /sys/fs/smackfs/load2 This went through the smack_uring_cmd and allowed the interaction. [1] : Here is the traceback of where smack prevents execution of the ioctl call: #0 smk_tskacc (tsp=0xffff888107a27300, obj_known=0xffff888105dda540, mode=mode@entry=2, a=a@entry=0xffffc90000c3be80) at ../security/smack/smack_access.c:258 #1 0xffffffff8143fbb0 in smk_curacc (obj_known=<optimized out>, mode=mode@entry=2, a=a@entry=0xffffc90000c3be80) at ../security/smack/smack_access.c:278 #2 0xffffffff8143b4e4 in smack_file_ioctl (file=<optimized out>, cmd=3225964097, arg=<optimized out>) at ../security/smack/smack_lsm.c:1539 #3 0xffffffff81411c3f in security_file_ioctl (file=file@entry=0xffff8881038c8b00, cmd=cmd@entry=3225964097, arg=arg@entry=140728408424048) at ../security/security.c:1552 #4 0xffffffff8126ca3e in __do_sys_ioctl (arg=140728408424048, cmd=3225964097, fd=3) at ../fs/ioctl.c:864 #5 __se_sys_ioctl (arg=140728408424048, cmd=3225964097, fd=3) at ../fs/ioctl.c:856 #6 __x64_sys_ioctl (regs=<optimized out>) at ../fs/ioctl.c:856 #7 0xffffffff81da0978 in do_syscall_x64 (nr=<optimized out>, regs=0xffffc90000c3bf58) at ../arch/x86/entry/common.c:50 #8 do_syscall_64 (regs=0xffffc90000c3bf58, nr=<optimized out>) at ../arch/x86/entry/common.c:80 #9 0xffffffff81e0009b in entry_SYSCALL_64 () at ../arch/x86/entry/entry_64.S:120 #10 0x00007f9c22ae9000 in ?? () #11 0x0000000000000000 in ?? () > > > Best > > > > Joel > > > > On Mon, Aug 29, 2022 at 09:20:09AM -0700, Casey Schaufler wrote: > >> 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. > >>>
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 };
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(+)