Thread (26 messages) 26 messages, 5 authors, 2019-02-13

Re: [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length

From: 'Marcelo Ricardo Leitner' <marcelo.leitner@gmail.com>
Date: 2019-02-07 17:47:24
Also in: linux-sctp, lkml

On Thu, Feb 07, 2019 at 05:33:07PM +0000, David Laight wrote:
From: Marcelo Ricardo Leitner
quoted
Sent: 06 February 2019 21:07

On Wed, Feb 06, 2019 at 12:48:38PM -0800, Julien Gomes wrote:
quoted

On 2/6/19 12:37 PM, Marcelo Ricardo Leitner wrote:
quoted
On Wed, Feb 06, 2019 at 12:14:30PM -0800, Julien Gomes wrote:
quoted
Make sctp_setsockopt_events() able to accept sctp_event_subscribe
structures longer than the current definitions.

This should prevent unjustified setsockopt() failures due to struct
sctp_event_subscribe extensions (as in 4.11 and 4.12) when using
binaries that should be compatible, but were built with later kernel
uapi headers.
Not sure if we support backwards compatibility like this?

My issue with this change is that by doing this, application will have
no clue if the new bits were ignored or not and it may think that an
event is enabled while it is not.

A workaround would be to do a getsockopt and check the size that was
returned. But then, it might as well use the right struct here in the
first place.

I'm seeing current implementation as an implicitly versioned argument:
it will always accept setsockopt calls with an old struct (v4.11 or
v4.12), but if the user tries to use v3 on a v1-only system, it will
be rejected. Pretty much like using a newer setsockopt on an old
system.
With the current implementation, given sources that say are supposed to
run on a 4.9 kernel (no use of any newer field added in 4.11 or 4.12),
we can't rebuild the exact same sources on a 4.19 kernel and still run
them on 4.9 without messing with structures re-definition.
Maybe what we want(ed) here then is explicit versioning, to have the 3
definitions available. Then the application is able to use, say struct
sctp_event_subscribe, and be happy with it, while there is struct
sctp_event_subscribe_v2 and struct sctp_event_subscribe_v3 there too.

But it's too late for that now because that would break applications
already using the new fields in sctp_event_subscribe.
It is probably better to break the recompilation of the few programs
that use the new fields (and have them not work on old kernels)
than to stop recompilations of old programs stop working on old
kernels or have requested new options silently ignored.
I got confused here, not sure what you mean. Seems there is one "stop"
word too many.
There are all sorts of reasons why programs get built on systems that
are newer than the ones they need to run on.
I'm currently planning to get around the glibc 'memcpy()' fubar so I
can retire some very old build systems before their disks die.
You can virtualize those. That's not really a good reason for
building with newer kernel and running on old systems, as virtually
any old system can be virtualized.

  Marcelo
Fortunately our sctp code is in the kernel - so has to be compiled
with the correct headers.
quoted
quoted
I understand your point, but this still looks like a sort of uapi
breakage to me.
Not disagreeing. I really just don't know how supported that is.
Willing to know so I can pay more attention to this on future changes.
Agreed, these structures should never be changed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help