lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 6 Dec 2023 06:53:10 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Luis Chamberlain <mcgrof@...nel.org>, 
	Joel Granados <j.granados@...sung.com>
Cc: Kees Cook <keescook@...omium.org>, 
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, Iurii Zaikin <yzaikin@...gle.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 13/18] sysctl: move sysctl type to ctl_table_header

On 2023-12-05 14:50:01-0800, Luis Chamberlain wrote:
> On Tue, Dec 5, 2023 at 2:41 PM Thomas Weißschuh <linux@...ssschuh.net> wrote:
> >
> > On 2023-12-05 14:33:38-0800, Luis Chamberlain wrote:
> > > On Mon, Dec 04, 2023 at 08:52:26AM +0100, Thomas Weißschuh wrote:
> > > > @@ -231,7 +231,8 @@ static int insert_header(struct ctl_dir *dir, struct ctl_table_header *header)
> > > >             return -EROFS;
> > > >
> > > >     /* Am I creating a permanently empty directory? */
> > > > -   if (sysctl_is_perm_empty_ctl_header(header)) {
> > > > +   if (header->ctl_table == sysctl_mount_point ||
> > > > +       sysctl_is_perm_empty_ctl_header(header)) {
> > > >             if (!RB_EMPTY_ROOT(&dir->root))
> > > >                     return -EINVAL;
> > > >             sysctl_set_perm_empty_ctl_header(dir_h);
> > >
> > > While you're at it.
> >
> > This hunk is completely gone in v3/the code that you merged.
> 
> It is worse in that it is not obvious:
> 
> +       if (table == sysctl_mount_point)
> +               sysctl_set_perm_empty_ctl_header(head);
> 
> > Which kind of unsafety do you envision here?
> 
> Making the code obvious during patch review hy this is needed /
> special, and if we special case this, why not remove enum, and make it
> specific to only that one table. The catch is that it is not
> immediately obvious that we actually call
> sysctl_set_perm_empty_ctl_header() in other places, and it begs the
> question if this can be cleaned up somehow.

Making it specific won't work because the flag needs to be transferred
from the leaf table to the table representing the directory.

What do you think of the aproach taken in the attached patch?
(On top of current sysctl-next, including my series)

Note: Current sysctl-next ist still based on v6.6.

View attachment "0001-sysctl-simplify-handling-of-permanently-empty-direct.patch" of type "text/plain" (7476 bytes)

Powered by blists - more mailing lists