Re: [uml-devel] [PATCH 1/1] hostfs: fix UML crash

[Geert Uytterhoeven]

On Mon, Oct 18, 2010 at 21:31, Richard Weinberger <richard@xxxxxx> wrote:
> Am Montag 18 Oktober 2010, 21:22:31 schrieb Geert Uytterhoeven:
>> On Mon, Oct 18, 2010 at 20:40, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> wrote:
>> > On Mon, 18 Oct 2010 18:36:54 +0200 Richard Weinberger <richard@xxxxxx>
> wrote:
>> >> 365b1818 resized f_spare within struct statfs.
>> >> hostfs accesses f_spare directly and needs an update.
>> >>
>> >> Signed-off-by: Richard Weinberger <richard@xxxxxx>
>> >> Reported-by: Toralf F__rster <toralf.foerster@xxxxxx>
>> >> Tested-by: Toralf F__rster <toralf.foerster@xxxxxx>
>> >> ---
>> >> Âfs/hostfs/hostfs_user.c | Â Â2 +-
>> >> Â1 files changed, 1 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/fs/hostfs/hostfs_user.c b/fs/hostfs/hostfs_user.c
>> >> index 6777aa0..ce2f168 100644
>> >> --- a/fs/hostfs/hostfs_user.c
>> >> +++ b/fs/hostfs/hostfs_user.c
>> >> @@ -388,6 +388,6 @@ int do_statfs(char *root, long *bsize_out, long long
>> >> *blocks_out, spare_out[1] = buf.f_spare[1];
>> >> Â Â Â spare_out[2] = buf.f_spare[2];
>> >> Â Â Â spare_out[3] = buf.f_spare[3];
>> >> - Â Â spare_out[4] = buf.f_spare[4];
>> >> +
>> >> Â Â Â return 0;
>> >> Â}
>> >
>> > Thanks.
>> >
>> > Is there any reason for hostfs to be playing with the f_spare field at
>> > all?
>>
>> It just copies it from struct statfs64 on the host to struct kstatfs
>> on the guest.
>> Probably a memcpy() is more future-safe, if it's combined with a
>> BUILD_BUG_ON(sizeof(statfs64.f_spare) != sizeof(kstatfs.f_spare)).
>>
>> Still, currently it doesn't copy the recently added f_flags field.
>> To protect against future changes like that, an explicit
>> BUILD_BUG_ON(sizeof(kstatfs.f_spare) != 4*sizeof(long)) may be even
>> better...
>
> Anyway, why do we need to copy f_spare from the host to the guest?
> I'm quite sure it can be omitted.

I guess it wants to preserve fields that are added in the future,
which may be useful
if the host is more recent than the guest.

Still, even if you don't want to preserve them, you probably want to clear them,
so memset() is better than a fixed loop through 4 elements.

Gr{oetje,eeting}s,

            Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
             Â Â -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/