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.
