netdev - RE: [PATCH net] netpoll: fix rx_hook() interface by passing the skb

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:	Wed, 23 Oct 2013 09:33:49 +0100
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Antonio Quartulli" <antonio@...hcoding.com>
Cc:	"David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: RE: [PATCH net] netpoll: fix rx_hook() interface by passing the skb

...
> > I can't remember which value you passed as 'offset' (and my mailer makes
> > it hard to find), but to ease the code changes the offset of the udp data
> > would make sense.
> > In that case you still need to pass the source port.
> 
> I decided not to pass the source port because if the user is really interested
> in it, it is still possible to get the udp_hdr from the skb and read its value.

It just seemed that there was no need to require that the hook re-parse
the ip header just to find the source port.
(ok it could assume that the udp header is just before the data)

> > If you do rx_hook(np, source_port, skb, offset) then if anyone manages to
> > load an old module (or code that casts the assignement to rx_poll)
> > at least it won't go 'bang'.
> > Renaming the structure member will guarantee to generate compile errors.
> 
> so you suggest to rename rx_hook to something else to warn people about the
> change?

Yes.

> If we go for the "no udp port" approach they will get an error any way because
> of the mismatching arguments.

No - you only get a warning when you assign a function pointer of the wrong type.
And that is true even if you just change the type of the pointer.
However code might already have a cast on the function pointer (eg because the
hook has 'unsigned char *') - so you won't even get a warning.
You then get an OOPS when the hook tries to read the buffer.

It is a really bad interface...
There isn't even a flags/options (etc) word that can be used
to detect enhancements.

	David