On Wed, Jun 15, 2022 at 4:03 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Tue, Jun 14, 2022 at 9:25 AM Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> wrote: > > > > In this function, it directly returns the result of __security_read_policy > > without freeing the allocated memory in *data, cause memory leak issue, > > so free the memory if __security_read_policy failed. > > > > Signed-off-by: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> > > --- > > security/selinux/ss/services.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > This is another case where there is not actually a memory leak as the > only caller of security_read_policy() is sel_open_policy() which will > free the buffer it passes to security_read_policy() on error. > > If you want you could add a comment to security_read_policy() > indicating that the caller is responsible for freeing the memory. Can we please not have two almost identical functions with different cleanup conventions? Please let's either make both functions guarantee cleanup on error or neither of them (adapting the caller(s) and comments accordingly). -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
