| Message ID | 20220617094412.197479-1-xiujianfeng@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [RESEND,-next] selinux: Let the caller free the momory in *mnt_opts on error | expand |
On Fri, Jun 17, 2022 at 5:46 AM Xiu Jianfeng <xiujianfeng@huawei.com> wrote: > > It may allocate memory for @mnt_opts if NULL in selinux_add_opt(), and > now some error paths goto @err label to free memory while others don't, > as suggested by Paul, don't free memory in case of error and let the > caller to cleanup on error. > > And also this patch changes the @s NULL check to return -EINVAL instead. > > Link: https://lore.kernel.org/lkml/20220611090550.135674-1-xiujianfeng@huawei.com/T/ > Suggested-by: Paul Moore <paul@paul-moore.com> > Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> > --- > security/selinux/hooks.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) Thanks, merged into selinux/next with some rewording of the subject line and commit description.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4d20a139a86d..9d08b91e05a2 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -944,10 +944,12 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, return rc; } +/* + * NOTE: the caller is resposible for freeing the memory even if on error. + */ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; - bool is_alloc_opts = false; u32 *dst_sid; int rc; @@ -955,7 +957,7 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) /* eaten and completely ignored */ return 0; if (!s) - return -ENOMEM; + return -EINVAL; if (!selinux_initialized(&selinux_state)) { pr_warn("SELinux: Unable to set superblock options before the security server is initialized\n"); @@ -967,7 +969,6 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) if (!opts) return -ENOMEM; *mnt_opts = opts; - is_alloc_opts = true; } switch (token) { @@ -1002,10 +1003,6 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) return rc; err: - if (is_alloc_opts) { - kfree(opts); - *mnt_opts = NULL; - } pr_warn(SEL_MOUNT_FAIL_MSG); return -EINVAL; }
It may allocate memory for @mnt_opts if NULL in selinux_add_opt(), and now some error paths goto @err label to free memory while others don't, as suggested by Paul, don't free memory in case of error and let the caller to cleanup on error. And also this patch changes the @s NULL check to return -EINVAL instead. Link: https://lore.kernel.org/lkml/20220611090550.135674-1-xiujianfeng@huawei.com/T/ Suggested-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> --- security/selinux/hooks.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)