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

Bug#1050256: AppArmor breaks locking non-fs Unix sockets



Hi John,

On Sun, Jan 28, 2024 at 12:43:33AM -0800, John Johansen wrote:
> On 12/30/23 20:24, Mathias Gibbens wrote:
> > On Sat, 2023-12-30 at 16:44 +0100, Salvatore Bonaccorso wrote:
> > > John, did you had a chance to work on this backport for 6.1.y stable
> > > upstream so we could pick it downstream in Debian in one of the next
> > > stable imports? Cherry-picking 1cf26c3d2c4c ("apparmor: fix apparmor
> > > mediating locking non-fs unix sockets") does not work, if not
> > > havinging the work around e2967ede2297 ("apparmor: compute policydb
> > > permission on profile load") AFAICS, so that needs a 6.1.y specific
> > > backport submitted to stable@vger.kernel.org ?
> > > 
> > > I think we could have people from this bug as well providing a
> > > Tested-by when necessary. I'm not feeling confident enough to be able
> > > to provide myself such a patch to sent to stable (and you only giving
> > > an Acked-by/Reviewed-by), so if you can help out here with your
> > > upstream hat on that would be more than appreciated and welcome :)
> > > 
> > > Thanks a lot for your work!
> > 
> >    I played around with this a bit the past week as well, and came to
> > the same conclusion as Salvatore did that commits e2967ede2297 and
> > 1cf26c3d2c4c need to be cherry-picked back to the 6.1 stable tree.
> > 
> >    I've attached the two commits rebased onto 6.1.y as patches to this
> > message. Commit e2967ede2297 needed a little bit of touchup to apply
> > cleanly, and 1cf26c3d2c4c just needed adjustments for line number
> > changes. I included some comments at the top of each patch.
> > 
> >    With these two commits cherry-picked on top of the 6.1.69 kernel, I
> > can boot a bookworm system and successfully start a service within a
> > container that utilizes `PrivateNetwork=yes`. Rebooting back into an
> > unpatched vanilla 6.1.69 kernel continues to show the problem.
> > 
> >    While I didn't see any immediate issues (ie, `aa-status` and log
> > files looked OK), I don't understand the changes in the first commit
> > well enough to be confident in sending these patches for inclusion in
> > the upstream stable tree on my own.
> > 
> > Mathias
> 
> Your backports look good to me, and you can stick my acked-by on them.
> The changes are strictly more than necessary for the fix. They are
> part of a larger change set that is trying to cleanup the runtime
> code by changing the permission mapping from a runtime operation
> to something that is done only at policy load/unpack time.
> 
> The advantage of this approach is that while it is a larger change
> than strictly necessary. It is backporting patches that are already
> upstream, keep the code closer and making backports easier.
> 
> Georgia did a minimal backport fix by keeping the version as part
> of policy and doing the permission mapping at runtime. I have
> included that patch below. Its advantage is it is a minimal
> change to fix the issue.
> 
> I am happy with either version going into stable. Do you want to
> send them or do you want me to do it?
> 
> Acked-by: John Johansen <john.johansen@canonical.com>

Thanks a lot, that is *really* much appreicated!

if you can send them that would be great, because think then they come
directly from you, the trust from Greg or Sasha is higher. otherwise I
think they will then explicitly want an ack on that submission thread
from you (or pointing to this Debian downstream bug).

Greg will probably want the backport apporach of the two commits if it
feasible and we do not expect regression from it. But you are
definitively in a better position to judge this :)

Thanks again!

Regards,
Salvatore

p.s.: feel free to CC us as well in the upstream stable submission.


Reply to: