On Mon, May 4, 2026 at 9:25 AM Tyler Hicks <[email protected]> wrote: > > On 2026-05-04 18:17:05, Zygmunt Krynicki wrote: > > > > > > W dniu 4.05.2026 o 18:13 Tyler Hicks pisze: > > > > >> FLAG_HIDDEN_UNCONFINED); > > >> if (len < 0) { > > >> + kfree(*string); > > >> + *string = NULL; > > > > > > Upstream doesn't have this call to kfree(). Did you create this patch > > > from an Ubuntu kernel tree? > > > > The kfree is coming from my patch. > > > > I think those are all against recent (at most weekend away) upstream master. > > Yes, I totally misread the trivial patch. My bad! > > John, please feel free to add: > > Fixes: 76a1d263aba3 ("apparmor: switch getprocattr to using label_print > fns()") > Reviewed-by: Tyler Hicks <[email protected]> > > Tyler > > > > > Best regards > > ZK >
The call of aa_getprocattr in apparmor_getselfattr unconditionally frees the returned string (which doesn't break with this patch applied because kfree(NULL) is a no-op), while the call in apparmor_getprocattr does result in a memory leakage if this case is not handled correctly. Even if one of the callsites would be fine, I believe it makes more sense to set the pointer to NULL inside aa_getprocattr instead of having its callers handle the freeing in the error case. (I also checked that the single caller of the security_getprocattr LSM hook, which apparmor_getprocattr is an implementation of, would be able to handle the pointer being set to NULL.) As such: Reviewed-by: Ryan Lee <[email protected]>
