netdev - Re: [PATCH ethtool 2/2] ethtool: remove restriction on ioctl commands having JSON output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Oct 2022 17:23:51 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Mathew McBride <matt@...verse.com.au>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH ethtool 2/2] ethtool: remove restriction on ioctl
 commands having JSON output

On Mon, Jul 04, 2022 at 05:41:14AM +0000, Mathew McBride wrote:
> The module-info/dump eeprom (-m/--module-info) is one such command
> which is now able to produce JSON.
> 
> Signed-off-by: Mathew McBride <matt@...verse.com.au>

I'm sorry for the delay, this came while I was on vacation so I needed
to think a bit more about JSON output in ioctl path, then it somehow
fell through the cracks.

> ---
>  ethtool.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 83bbde8..ece4ac0 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6409,9 +6409,6 @@ int main(int argc, char **argp)
>  	ctx.argp = argp;
>  	netlink_run_handler(&ctx, args[k].nlchk, args[k].nlfunc, !args[k].func);
>  
> -	if (ctx.json) /* no IOCTL command supports JSON output */
> -		exit_bad_args();
> -
>  	ret = ioctl_init(&ctx, args[k].no_dev);
>  	if (ret)
>  		return ret;

I don't think this is a good idea. This way you effectively revert
commit 9a935085ec1c ("ethtool: return error if command does not support
--json") in ioctl path so that --json will be silently ignored there for
all commands which do not support JSON output, i.e. for all except one.

One way around would be adding an extra flag to mark commands supporting
JSON output in ioctl path but IMHO it's not likely that there are going
to be many more. It would be probably easier keep the check as is.

Michal

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)