* Unmerged patches adding audit when protected_regular/fifos sysctl causes EACCES @ 2019-09-25 18:58 Jérémie Galarneau 2019-09-25 20:12 ` Kees Cook 0 siblings, 1 reply; 3+ messages in thread From: Jérémie Galarneau @ 2019-09-25 18:58 UTC (permalink / raw) To: keescook Cc: s.mesoraca16, viro, dan.carpenter, akpm, Mathieu Desnoyers, lkml, torvalds, solar Hi Kees, I have noticed that the two top-most patches of your protected-creat branch were never merged upstream [1]. Those patches add audit logs whenever the protected_regular or protected_fifo sysctl prevent the creation of a file/fifo. They were mentioned in the v4 thread [2] of the "main" patch and seemed acceptable, but they were no longer mentioned in v5 [3], which was merged. Now that systemd enables those sysctls by default (v241+), I got bitten pretty hard by this check and it took me a while to figure out what was happening [4]. I ended up catching it by adding a bunch of printk(), including where you proposed to add an audit log statement. I just found your two patches while implementing what you proposed almost 1:1. Was there a reason why those were abandoned? Otherwise, would you mind resubmitting them? Thanks! Jérémie [1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/userspace/protected-creat [2] https://lkml.org/lkml/2018/4/10/840 [3] https://lore.kernel.org/lkml/20180416175918.GA13494@beast/ [4] https://github.com/lttng/lttng-tools/commit/cf86ff2c4ababd01fea7ab2c9c289cb7c0a1bcd5 -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Unmerged patches adding audit when protected_regular/fifos sysctl causes EACCES 2019-09-25 18:58 Unmerged patches adding audit when protected_regular/fifos sysctl causes EACCES Jérémie Galarneau @ 2019-09-25 20:12 ` Kees Cook 2019-09-25 20:23 ` Mathieu Desnoyers 0 siblings, 1 reply; 3+ messages in thread From: Kees Cook @ 2019-09-25 20:12 UTC (permalink / raw) To: Jérémie Galarneau Cc: s.mesoraca16, viro, dan.carpenter, akpm, Mathieu Desnoyers, lkml, torvalds, solar On Wed, Sep 25, 2019 at 02:58:28PM -0400, Jérémie Galarneau wrote: > Hi Kees, > > I have noticed that the two top-most patches of your protected-creat > branch were never merged upstream [1]. Those patches add audit logs > whenever the protected_regular or protected_fifo sysctl prevent the > creation of a file/fifo. > > They were mentioned in the v4 thread [2] of the "main" patch and > seemed acceptable, but they were no longer mentioned in v5 [3], which > was merged. > > Now that systemd enables those sysctls by default (v241+), I got > bitten pretty hard by this check and it took me a while to figure out > what was happening [4]. I ended up catching it by adding a bunch of > printk(), including where you proposed to add an audit log statement. > > I just found your two patches while implementing what you proposed almost 1:1. > > Was there a reason why those were abandoned? Otherwise, would you mind > resubmitting them? Hi! There was concern about getting buy-in from the audit folks delaying things even more. Instead of waiting for that, as it had already taken a long time to get consensus even on the functionality, they were dropped. I'll rebase them and send them out again; thanks for the ping! -Kees > > Thanks! > Jérémie > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/userspace/protected-creat > [2] https://lkml.org/lkml/2018/4/10/840 > [3] https://lore.kernel.org/lkml/20180416175918.GA13494@beast/ > [4] https://github.com/lttng/lttng-tools/commit/cf86ff2c4ababd01fea7ab2c9c289cb7c0a1bcd5 > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com -- Kees Cook ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Unmerged patches adding audit when protected_regular/fifos sysctl causes EACCES 2019-09-25 20:12 ` Kees Cook @ 2019-09-25 20:23 ` Mathieu Desnoyers 0 siblings, 0 replies; 3+ messages in thread From: Mathieu Desnoyers @ 2019-09-25 20:23 UTC (permalink / raw) To: Kees Cook Cc: Jeremie Galarneau, s mesoraca16, Alexander Viro, dan carpenter, Andrew Morton, linux-kernel, Linus Torvalds, solar ----- On Sep 25, 2019, at 4:12 PM, Kees Cook keescook@chromium.org wrote: > On Wed, Sep 25, 2019 at 02:58:28PM -0400, Jérémie Galarneau wrote: >> Hi Kees, >> >> I have noticed that the two top-most patches of your protected-creat >> branch were never merged upstream [1]. Those patches add audit logs >> whenever the protected_regular or protected_fifo sysctl prevent the >> creation of a file/fifo. >> >> They were mentioned in the v4 thread [2] of the "main" patch and >> seemed acceptable, but they were no longer mentioned in v5 [3], which >> was merged. >> >> Now that systemd enables those sysctls by default (v241+), I got >> bitten pretty hard by this check and it took me a while to figure out >> what was happening [4]. I ended up catching it by adding a bunch of >> printk(), including where you proposed to add an audit log statement. >> >> I just found your two patches while implementing what you proposed almost 1:1. >> >> Was there a reason why those were abandoned? Otherwise, would you mind >> resubmitting them? > > Hi! > > There was concern about getting buy-in from the audit folks delaying > things even more. Instead of waiting for that, as it had already taken > a long time to get consensus even on the functionality, they were > dropped. > > I'll rebase them and send them out again; thanks for the ping! If you need additional justification for why those are needed, here are a few problematic scenarios we're observing in the current situation. Feel free to use those if you need to add extra justification for your audit patches commit messages. A first scenario is a host with containers, where a container runs userspace processes which depend on the open() behavior changed by those sysctl. If the host is updated to systemd 241+, which enables those sysctl by default, those containers will start misbehaving, and figuring out the culprit without any hint in the kernel dmesg is far from obvious. A similar situation happens for non-containerized deployments. If an application depends on this open() ABI behavior tweaked by those sysctl, the application will start failing if it happens to run on a system with systemd 241+. Again, without any dmesg printout, it's rather hard to diagnose. Thanks! Mathieu > > -Kees > >> >> Thanks! >> Jérémie >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/userspace/protected-creat >> [2] https://lkml.org/lkml/2018/4/10/840 >> [3] https://lore.kernel.org/lkml/20180416175918.GA13494@beast/ >> [4] >> https://github.com/lttng/lttng-tools/commit/cf86ff2c4ababd01fea7ab2c9c289cb7c0a1bcd5 >> >> -- >> Jérémie Galarneau >> EfficiOS Inc. >> http://www.efficios.com > > -- > Kees Cook -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-25 20:24 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-25 18:58 Unmerged patches adding audit when protected_regular/fifos sysctl causes EACCES Jérémie Galarneau 2019-09-25 20:12 ` Kees Cook 2019-09-25 20:23 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox