On Sun, Apr 04, 2021 at 11:14:32AM +0300, Danielle Ratson wrote:
> Some drivers clear the 'ethtool_link_ksettings' struct in their
> get_link_ksettings() callback, before populating it with actual values.
> Such drivers will set the new 'link_mode' field to zero, resulting in
> user space receiving wrong link mode information given that zero is a
> valid value for the field.
That sounds like the bug, that 0 means something, not that the
structure is zeroed. It is good practice to zero structures about to
be returned to user space otherwise you could leak stack information
etc.
Do we still have time to fix the KAPI, so zero means unset? Where in
the workflow is the patch adding this feature? Is it in a released
kernel? or -rc kernel?
> Fix this by introducing a new capability bit ('cap_link_mode_supported')
> to ethtool_ops, which indicates whether the driver supports 'link_mode'
> parameter or not. Set it to true in mlxsw which is currently the only
> driver supporting 'link_mode'.
>
> Another problem is that some drivers (notably tun) can report random
> values in the 'link_mode' field. This can result in a general protection
> fault when the field is used as an index to the 'link_mode_params' array
> [1].
>
> This happens because such drivers implement their set_link_ksettings()
> callback by simply overwriting their private copy of
> 'ethtool_link_ksettings' struct with the one they get from the stack,
> which is not always properly initialized.
>
> Fix this by making sure that the implied parameters will be derived from
> the 'link_mode' parameter only when the 'cap_link_mode_supported' is set.
So you left in place the kernel memory leak to user space? Or is there
an additional patch to fix tun as well?
Andrew
