Re: [PATCH v9 00/11] landlock: truncate support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 17, 2022 at 5:16 AM Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
> On 16/10/2022 22:07, Günther Noack wrote:

...

> > Proposed fix
> > ------------
> >
> > I think the LSM framework should ensure that security blobs are
> > pointer-aligned.
> >
> > The LSM framework takes the role of a memory allocator here, and
> > memory allocators should normally return aligned addresses, in my
> > understanding. -- It seems reasonable for AppArmor to make that
> > assumption.
> >
> > The proposed one-line fix is: Change lsm_set_blob_size() in
> > security/security.c, where the positions of the individual security
> > blobs are calculated, so that each allocated blob is aligned to a
> > pointer size boundary.
> >
> > if (*need > 0) {
> >    *lbs = ALIGN(*lbs, sizeof(void *));   // NEW
> >
> >    offset = *lbs;
> >    *lbs += *need;
> >    *need = offset;
> > }
>
> This looks good to me. This fix should be part of patch 4/11 since it
> only affects Landlock for now.

Hi Günther,

Sorry for not seeing this email sooner; I had thought the landlock
truncate work was largely resolved with just a few small things for
you to sort out with Mickaël so I wasn't following this thread very
closely anymore.

Regarding the fix, yes, I think the solution is to fixup the LSM
security blob allocator to properly align the entries.  As you already
mentioned, that's common behavior elsewhere and I see no reason why we
should deviate from that in the LSM allocator.  Honestly, looking at
the rest of the allocator right now I can see a few other things to
improve, but those can wait for a later time so as to not conflict
with this work (/me adds a new entry to my todo list).

Other than that, I might suggest the lsm_set_blob_size()
implementation below as it seems cleaner to me and should be
functionally equivalent ... at least on quick inspection, if I've done
something dumb with the code below please feel free to ignore me ;)

  static void __init lsm_set_blob_size(int *need, int *lbs)
  {
    if (*need <= 0)
      return;

    *need = ALIGN(*need, sizeof(void *));
    *lbs += *need;
  }

-- 
paul-moore.com




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux