On Sun, Feb 17, 2019 at 03:41:21AM +0000, Arthur Gautier wrote:
> On Sat, Feb 16, 2019 at 11:47:02PM +0000, Al Viro wrote:
> > On Sat, Feb 16, 2019 at 02:50:15PM -0800, Andy Lutomirski wrote:
> >
> > > What is the actual problem? We’re not actually demand-faulting this
> > > data, are we? Are we just overrunning the buffer because the from_user
> > > helpers are too clever? Can we fix it for real by having the fancy
> > > helpers do *aligned* loads so that they don’t overrun the buffer? Heck,
> > > this might be faster, too.
> >
> > Unaligned _stores_ are not any cheaper, and you'd get one hell of
> > extra arithmetics from trying to avoid both. Check something
> > like e.g. memcpy() on alpha, where you really have to keep all
> > accesses aligned, both on load and on store side.
> >
> > Can't we just pad the buffers a bit? Making sure that name_buf
> > and symlink_buf are _not_ followed by unmapped pages shouldn't
> > be hard. Both are allocated by kmalloc(), so...
>
> We cannot change alignment rules here. The input buffer string we're
> reading is coming from an cpio formated file and the format is
> defined by cpio(5).
> Nothing much we can do there I'm afraid. Input buffer is defined to
> be 4-byte aligned.
Who says anything about changing the format of the file? At least
one trivial way to handle that would be this:
diff --git a/init/initramfs.c b/init/initramfs.c
index 7cea802d00ef..edbddfb73106 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -265,8 +265,12 @@ static int __init do_header(void)
state = Collect;
return 0;
}
- if (S_ISREG(mode) || !body_len)
- read_into(name_buf, N_ALIGN(name_len), GotName);
+ if (S_ISREG(mode) || !body_len) {
+ collect = collected = name_buf;
+ remains = N_ALIGN(name_len);
+ next_state = GotName;
+ state = Collect;
+ }
return 0;
}
Another would be to have the buffer passed to flush_buffer() (i.e.
the callback of decompress_fn) allocated with 4 bytes of padding
past the part where the unpacked piece of data is placed for the
callback to find. As in,
diff --git a/lib/decompress_inflate.c b/lib/decompress_inflate.c
index 63b4b7eee138..ca3f7ecc9b35 100644
--- a/lib/decompress_inflate.c
+++ b/lib/decompress_inflate.c
@@ -48,7 +48,7 @@ STATIC int INIT __gunzip(unsigned char *buf, long len,
rc = -1;
if (flush) {
out_len = 0x8000; /* 32 K */
- out_buf = malloc(out_len);
+ out_buf = malloc(out_len + 4);
} else {
if (!out_len)
out_len = ((size_t)~0) - (size_t)out_buf; /* no limit */
for gunzip/decompress and similar ones for bzip2, etc. The contents
layout doesn't have anything to do with that...
