netdev - Re: [PATCH 1/2] netlink: add NLA

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:   Mon, 17 Sep 2018 17:17:29 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     Michal Kubecek <mkubecek@...e.cz>, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, jbenc@...hat.com
Subject: Re: [PATCH 1/2] netlink: add NLA_REJECT policy type

On Mon, Sep 17, 2018 at 11:38:52AM +0200, Johannes Berg wrote:
> On Thu, 2018-09-13 at 18:58 -0300, Marcelo Ricardo Leitner wrote:
> 
> > Then I ask my first question again: why reject these? They are not
> > hurting anything, are they?  It's different from your example I think.
> > In there, the extra information which was ignored leads to a
> > different behavior.
> 
> So in one case I was thinking of, there are some fields that simply
> cannot be used for input, they're only used for output. But it may not
> always be obvious to somebody using the API. Thus, I think it makes
> sense to instruct the kernel to reject that, so that whoever gets
> confused has immediate feedback that their usage is wrong. If we ignore
> that, they may not realize their error immediately.
> 
> I think the ethtool case is similar: you can read and write some fields,
> and only read others - but if you try to write the read-only fields
> would you prefer to be told "sorry, this is not possible" vs. it being
> silently ignored? I'd definitely prefer the former.

See below.

> 
> > Maybe it would be better to have NLA_IGNORE instead? </idea>
> 
> I don't think so, it doesn't give any feedback to the application author
> that they're doing something wrong.

Yes, it would have to have some other ways to signal return values.

In some cases there may be other ways to tell the application that it
couldn't be handled at the time. For example, when changing several
ethtool offloadings at once, we could have a feedback for each of the
offloading that was request and it could include "ack, nack and
ignored" and let the application decide if that "ignored" should be an
error or not. It all boils down to what one is trying to do.

That said, I'm now liking this patch. Just too bad we cannot flag
current output-only fields as so, but in the long term I think this
patch will help us.

Thanks,
  Marcelo