| Message ID | 20210512143210.248684-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v2] lsm_audit,selinux: pass IB device name by reference | expand |
On 2021-05-12 16:32, Ondrej Mosnacek wrote: > While trying to address a Coverity warning that the dev_name string > might end up unterminated when strcpy'ing it in > selinux_ib_endport_manage_subnet(), I realized that it is possible (and > simpler) to just pass the dev_name pointer directly, rather than copying > the string to a buffer. > > The ibendport variable goes out of scope at the end of the function > anyway, so the lifetime of the dev_name pointer will never be shorter > than that of ibendport, thus we can safely just pass the dev_name > pointer and be done with it. Agreed, this is better (other than the unrelated whitespace changes). > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> Acked-by: Richard Guy Briggs <rgb@redhat.com> > --- > include/linux/lsm_audit.h | 8 ++++---- > security/selinux/hooks.c | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) > > v2: just pass the dev_name pointer and avoid the string copy > > v1: > https://lore.kernel.org/selinux/20210507130445.145457-1-omosnace@redhat.com/T/ > > diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h > index cd23355d2271..17d02eda9538 100644 > --- a/include/linux/lsm_audit.h > +++ b/include/linux/lsm_audit.h > @@ -48,13 +48,13 @@ struct lsm_ioctlop_audit { > }; > > struct lsm_ibpkey_audit { > - u64 subnet_prefix; > - u16 pkey; > + u64 subnet_prefix; > + u16 pkey; > }; > > struct lsm_ibendport_audit { > - char dev_name[IB_DEVICE_NAME_MAX]; > - u8 port; > + const char *dev_name; > + u8 port; > }; > > /* Auxiliary data to use in generating the audit record. */ > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 92f909a2e8f7..4d9764dad18f 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -6864,7 +6864,7 @@ static int selinux_ib_endport_manage_subnet(void *ib_sec, const char *dev_name, > return err; > > ad.type = LSM_AUDIT_DATA_IBENDPORT; > - strncpy(ibendport.dev_name, dev_name, sizeof(ibendport.dev_name)); > + ibendport.dev_name = dev_name; > ibendport.port = port_num; > ad.u.ibendport = &ibendport; > return avc_has_perm(&selinux_state, > -- > 2.31.1 > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://listman.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
On Wed, May 12, 2021 at 10:32 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > While trying to address a Coverity warning that the dev_name string > might end up unterminated when strcpy'ing it in > selinux_ib_endport_manage_subnet(), I realized that it is possible (and > simpler) to just pass the dev_name pointer directly, rather than copying > the string to a buffer. > > The ibendport variable goes out of scope at the end of the function > anyway, so the lifetime of the dev_name pointer will never be shorter > than that of ibendport, thus we can safely just pass the dev_name > pointer and be done with it. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > include/linux/lsm_audit.h | 8 ++++---- > security/selinux/hooks.c | 2 +- > 2 files changed, 5 insertions(+), 5 deletions(-) Much better, merged into selinux/next. Thanks.
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index cd23355d2271..17d02eda9538 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -48,13 +48,13 @@ struct lsm_ioctlop_audit { }; struct lsm_ibpkey_audit { - u64 subnet_prefix; - u16 pkey; + u64 subnet_prefix; + u16 pkey; }; struct lsm_ibendport_audit { - char dev_name[IB_DEVICE_NAME_MAX]; - u8 port; + const char *dev_name; + u8 port; }; /* Auxiliary data to use in generating the audit record. */ diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 92f909a2e8f7..4d9764dad18f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6864,7 +6864,7 @@ static int selinux_ib_endport_manage_subnet(void *ib_sec, const char *dev_name, return err; ad.type = LSM_AUDIT_DATA_IBENDPORT; - strncpy(ibendport.dev_name, dev_name, sizeof(ibendport.dev_name)); + ibendport.dev_name = dev_name; ibendport.port = port_num; ad.u.ibendport = &ibendport; return avc_has_perm(&selinux_state,
While trying to address a Coverity warning that the dev_name string might end up unterminated when strcpy'ing it in selinux_ib_endport_manage_subnet(), I realized that it is possible (and simpler) to just pass the dev_name pointer directly, rather than copying the string to a buffer. The ibendport variable goes out of scope at the end of the function anyway, so the lifetime of the dev_name pointer will never be shorter than that of ibendport, thus we can safely just pass the dev_name pointer and be done with it. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- include/linux/lsm_audit.h | 8 ++++---- security/selinux/hooks.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) v2: just pass the dev_name pointer and avoid the string copy v1: https://lore.kernel.org/selinux/20210507130445.145457-1-omosnace@redhat.com/T/