Re: [PATCH net-next] ethtool: copy reglen to userspace

From: Vivien Didelot
Date: Thu May 30 2019 - 19:42:25 EST


Hi Michal, David,

On Thu, 30 May 2019 11:26:30 -0700 (PDT), David Miller <davem@xxxxxxxxxxxxx> wrote:
> From: Michal Kubecek <mkubecek@xxxxxxx>
> Date: Thu, 30 May 2019 10:27:22 +0200
>
> > I believe this should be handled by ethtool_get_regs(), either by
> > returning an error or by only copying data up to original regs.len
> > passed by userspace. The former seems more correct but broken userspace
> > software would suddenly start to fail where it "used to work". The
> > latter would be closer to current behaviour but it would mean that
> > broken userspace software might nerver notice there is something wrong.
>
> I therefore think we need to meticulously fixup all of these adjustments
> of regs.len before adjusting the copy call here in generic ethtool.

Indeed there are cases where userspace may allocate a smaller buffer
(even though there's no way for the kernel to ensure the size that
was really allocated, but well...).

The kernel must still allocates a buffer of size ops->get_regs_len()
and pass it to the driver. It's OK if the kernel driver messes with
regs->len, since it's an open structure. Software may actually make
use of it. However the kernel must use the original regs->len passed
by userspace (up to ops->get_regs_len()) when copying the data.

I'm sending a new patch right away.


Thanks,
Vivien