Received-SPF: pass (google.com: domain of linux-kernel+bounces-70254-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Sun, 18 Feb 2024 08:47:03 +0100
From: Greg KH <gregkh@linuxfoundation.org>
To: Guixiong Wei <guixiongwei@gmail.com>
Cc: linux-hardening@vger.kernel.org, jgross@suse.com,
	boris.ostrovsky@oracle.com, linux-kernel@vger.kernel.org,
	Guixiong Wei <weiguixiong@bytedance.com>
Subject: Re: [RESEND RFC] kernel/ksysfs.c: restrict /sys/kernel/notes to root
 access
Message-ID: <2024021825-skiing-trustee-a56a@gregkh>
References: <20240218073501.54555-1-guixiongwei@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240218073501.54555-1-guixiongwei@gmail.com>

On Sun, Feb 18, 2024 at 03:35:01PM +0800, Guixiong Wei wrote:
> From: Guixiong Wei <weiguixiong@bytedance.com>
> 
> Restrict non-privileged user access to /sys/kernel/notes to
> avoid security attack.
> 
> The non-privileged users have read access to notes. The notes
> expose the load address of startup_xen. This address could be
> used to bypass KASLR.

How can it be used to bypass it?

KASLR is, for local users, pretty much not an issue, as that's not what
it protects from, only remote ones.

> For example, the startup_xen is built at 0xffffffff82465180 and
> commit_creds is built at 0xffffffff810ad570 which could read from
> the /boot/System.map. And the loaded address of startup_xen is
> 0xffffffffbc265180 which read from /sys/kernel/notes. So the loaded
> address of commit_creds is 0xffffffffbc265180 - (0xffffffff82465180
>  - 0xffffffff810ad570) = 0xffffffffbaead570.

I've cc: the hardening list on this, I'm sure the developers there have
opinions about this.

> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> ---
>  kernel/ksysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> index b1292a57c2a5..09bc0730239b 100644
> --- a/kernel/ksysfs.c
> +++ b/kernel/ksysfs.c
> @@ -199,7 +199,7 @@ static ssize_t notes_read(struct file *filp, struct kobject *kobj,
>  static struct bin_attribute notes_attr __ro_after_init  = {
>  	.attr = {
>  		.name = "notes",
> -		.mode = S_IRUGO,
> +		.mode = S_IRUSR,
>  	},
>  	.read = &notes_read,
>  };

No objection from me, but what userspace tool requires access to this
file today?  Will it break if permissions are changed on it?

And what about the module notes files?  If you change one, shouldn't you
change all?

thanks,

greg k-h