Re: [PATCH 1/5] x86/vm86: Move userspace accesses to do_sys_vm86()

[Andy Lutomirski]

On Fri, Jul 10, 2015 at 10:09 PM, Brian Gerst <brgerst@xxxxxxxxx> wrote:
> Move the userspace accesses down into the common function in
> preparation for the next set of patches.
>

One thing I don't like about the current code that makes these patches
harder to review is the bizarre approach to copying.  If you changed
this:

> -       tmp = copy_vm86_regs_from_user(&info.regs, &v86->regs,
> -                                      offsetof(struct kernel_vm86_struct, vm86plus) -
> -                                      sizeof(info.regs));

into a normal field-by-field get_user / copy_from_user (the latter for
the big regs struct) then it would be clear what the ABI is and it
would be much easier to read the patches and confirm that you aren't
accidentally changing the ABI.

You could also get rid of the constraint that certain fields in
apparently kernel-internal structs had to be in a certain order.

Other than that, patches 1-4 look good on cursory inspection.  I'll
look more carefully later.  I need to think about patch 5 more.

--Andy
--
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/