On Fri, 2022-11-18 at 08:44 -0500, Paul Moore wrote:
> On Thu, Nov 17, 2022 at 8:54 PM Serge E. Hallyn <serge@xxxxxxxxxx> wrote:
> > On Wed, Nov 09, 2022 at 11:36:14PM -0500, Paul Moore wrote:
> > > The vfs_getxattr_alloc() function currently returns a ssize_t value
> > > despite the fact that it only uses int values internally for return
> > > values. Fix this by converting vfs_getxattr_alloc() to return an
> > > int type and adjust the callers as necessary. As part of these
> > > caller modifications, some of the callers are fixed to properly free
> > > the xattr value buffer on both success and failure to ensure that
> > > memory is not leaked in the failure case.
>
> > > Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> >
> > Reviewed-by: Serge Hallyn <serge@xxxxxxxxxx>
> >
> > Do I understand right that the change to process_measurement()
> > will avoid an unnecessary call to krealloc() if the xattr has
> > not changed size between the two calls to ima_read_xattr()?
> > If something more than that is going on there, it might be
> > worth pointing out in the commit message.
>
> Yes, that was the intent, trying to avoid extra calls to krealloc().
>
> Mimi, have you had a chance to look at this patch yet? In addition to
> cleaning up the vfs_getxattr_alloc() function it resolves some issues
> with IMA (memory leaks), but as you're the IMA expert I really need
> your review on this ...b
All the other vfs_{get/set/list}xattr functions return ssize_t. Why
should vfs_getxattr_alloc() be any different?
The only time there could be a memory leak is when the
vfs_getxattr_alloc() caller provides a buffer which isn't large enough.
The one example in IMA/EVM is the call to evm_calc_hmac_or_hash(),
which is freeing the memory.
Perhaps I'm missing something, but from an IMA/EVM perspective, I see a
style change (common exit), but not any memory leak fixes. I'm fine
with the style change.
--
thanks,
Mimi
