Hi,
在 2022/6/14 4:22, Paul Moore 写道:
On Sat, Jun 11, 2022 at 5:07 AM Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> wrote:
In the entry of selinux_add_opt, *mnt_opts may be assigned to new
allocated memory, and also may be freed and reset at the end of the
function. however, if security_context_str_to_sid failed, it returns
directly and skips the procedure for free and reset, even if it may be
handled at the caller of this function, It is better to handle it
inside.
Fixes: 70f4169ab421 ("selinux: parse contexts for mount options early")
Signed-off-by: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx>
---
security/selinux/hooks.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Have you actually observed a memory leak from the selinux_mnt_opts
allocation in selinux_add_opt()?
The selinux_add_opt() function has two callers:
selinux_sb_eat_lsm_opts() and selinux_fs_context_parse_param(). The
former cleans up the selinux_mnt_opts allocation it its error handler
while the latter will end up calling
security_free_mnt_opts()/selinux_free_mnt_opts() to free the
fs_context:security when the fs_context is destroyed.
This patch shouldn't be necessary.
I may not have made it clear, I said potential means may have a third
caller in the future. Anyway,
Yes, you are right, currently no memleak here, because the two callers
will do the cleanup, from this point of view,
I think the error handler as following is not necessary:
err:
if (is_alloc_opts) {
kfree(opts);
*mnt_opts = NULL;
}
otherwise, some error paths goto err label while others don't, It's
confusing.
