Re: [PATCH] initramfs: avoid filename buffer overrun

[David Disseldorp <ddiss@suse.de>]

On Tue, 29 Oct 2024 18:35:20 +0000, Al Viro wrote:

> On Tue, Oct 29, 2024 at 12:48:37PM +0000, David Disseldorp wrote:
...
> > +	if (collected[name_len - 1] != '\0') {
> > +		pr_err("Skipping symlink without nulterm: %.*s\n",
> > +		       (int)name_len, collected);  
> 
> I'm not sure pr_err() and continue is a good approach here -
> you'd been given a corrupted image, so there's no point trying
> to do anything further with it.  Have it return 1, at least,
> and preferably use error("buggered symlink") in addition or
> instead of your pr_err().

I was following the name_len > PATH_MAX handling, but failing
immediately makes more sense here. Will change in v2.

> FWIW, it's _not_ about trying to stop an attack - if you get there with
> image contents controlled by attacker, you have already hopelessly lost;
> no buffer overruns are needed.
> 
> It does catch corrupted images, which is the right thing to do, but it's
> not a security issue.

Agreed. I'll rework the commit message to more clearly state that
initramfs image write access is required, at which point all bets are
off.