mbox series

[net-next,0/6] sctp: fix a plenty of flexible-array-nested warnings

Message ID cover.1681917361.git.lucien.xin@gmail.com (mailing list archive)
Headers show
Series sctp: fix a plenty of flexible-array-nested warnings | expand

Message

Xin Long April 19, 2023, 3:16 p.m. UTC
Paolo noticed a compile warning in SCTP,

../net/sctp/stream_sched_fc.c: note: in included file (through ../include/net/sctp/sctp.h):
../include/net/sctp/structs.h:335:41: warning: array of flexible structures

But not only this, there are actually quite a lot of such warnings in
some SCTP structs. This patchset fixes most of warnings by deleting
these nested flexible array members.

After this patchset, there are still some warnings left:

  # make C=2 CF="-Wflexible-array-nested" M=./net/sctp/
  ./include/net/sctp/structs.h:1145:41: warning: nested flexible array
  ./include/uapi/linux/sctp.h:641:34: warning: nested flexible array
  ./include/uapi/linux/sctp.h:643:34: warning: nested flexible array
  ./include/uapi/linux/sctp.h:644:33: warning: nested flexible array
  ./include/uapi/linux/sctp.h:650:40: warning: nested flexible array
  ./include/uapi/linux/sctp.h:653:39: warning: nested flexible array

the 1st is caused by __data[] in struct ip_options, not in SCTP;
the others are in uapi, and we should not touch them.

Note that instead of completely deleting it, we just leave it as a
comment in the struct, signalling to the reader that we do expect
such variable parameters over there, as Marcelo suggested.

Xin Long (6):
  sctp: delete the nested flexible array params
  sctp: delete the nested flexible array skip
  sctp: delete the nested flexible array variable
  sctp: delete the nested flexible array peer_init
  sctp: delete the nested flexible array hmac
  sctp: delete the nested flexible array payload

 include/linux/sctp.h         | 18 +++++++++---------
 include/net/sctp/sctp.h      | 12 ++++++------
 include/net/sctp/structs.h   |  2 +-
 net/sctp/associola.c         |  5 +++--
 net/sctp/auth.c              |  2 +-
 net/sctp/input.c             |  2 +-
 net/sctp/outqueue.c          | 11 +++++++----
 net/sctp/sm_make_chunk.c     | 22 +++++++++++-----------
 net/sctp/sm_sideeffect.c     |  3 +--
 net/sctp/sm_statefuns.c      | 14 ++++++--------
 net/sctp/stream.c            |  2 +-
 net/sctp/stream_interleave.c |  4 ++--
 12 files changed, 49 insertions(+), 48 deletions(-)

Comments

Jakub Kicinski April 20, 2023, 1:18 a.m. UTC | #1
On Wed, 19 Apr 2023 11:16:27 -0400 Xin Long wrote:
> Paolo noticed a compile warning in SCTP,
> 
> ../net/sctp/stream_sched_fc.c: note: in included file (through ../include/net/sctp/sctp.h):
> ../include/net/sctp/structs.h:335:41: warning: array of flexible structures
> 
> But not only this, there are actually quite a lot of such warnings in
> some SCTP structs. This patchset fixes most of warnings by deleting
> these nested flexible array members.
> 
> After this patchset, there are still some warnings left:
> 
>   # make C=2 CF="-Wflexible-array-nested" M=./net/sctp/
>   ./include/net/sctp/structs.h:1145:41: warning: nested flexible array
>   ./include/uapi/linux/sctp.h:641:34: warning: nested flexible array
>   ./include/uapi/linux/sctp.h:643:34: warning: nested flexible array
>   ./include/uapi/linux/sctp.h:644:33: warning: nested flexible array
>   ./include/uapi/linux/sctp.h:650:40: warning: nested flexible array
>   ./include/uapi/linux/sctp.h:653:39: warning: nested flexible array
> 
> the 1st is caused by __data[] in struct ip_options, not in SCTP;
> the others are in uapi, and we should not touch them.
> 
> Note that instead of completely deleting it, we just leave it as a
> comment in the struct, signalling to the reader that we do expect
> such variable parameters over there, as Marcelo suggested.

Hi Kees, is there no workaround for nested flexible arrays within 
the kernel?  Any recommendations?

https://lore.kernel.org/all/cover.1681917361.git.lucien.xin@gmail.com/
patchwork-bot+netdevbpf@kernel.org April 21, 2023, 7:50 a.m. UTC | #2
Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Wed, 19 Apr 2023 11:16:27 -0400 you wrote:
> Paolo noticed a compile warning in SCTP,
> 
> ../net/sctp/stream_sched_fc.c: note: in included file (through ../include/net/sctp/sctp.h):
> ../include/net/sctp/structs.h:335:41: warning: array of flexible structures
> 
> But not only this, there are actually quite a lot of such warnings in
> some SCTP structs. This patchset fixes most of warnings by deleting
> these nested flexible array members.
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] sctp: delete the nested flexible array params
    https://git.kernel.org/netdev/net-next/c/add7370a3989
  - [net-next,2/6] sctp: delete the nested flexible array skip
    https://git.kernel.org/netdev/net-next/c/73175a042955
  - [net-next,3/6] sctp: delete the nested flexible array variable
    https://git.kernel.org/netdev/net-next/c/9789c1c6619e
  - [net-next,4/6] sctp: delete the nested flexible array peer_init
    https://git.kernel.org/netdev/net-next/c/f97278ff346a
  - [net-next,5/6] sctp: delete the nested flexible array hmac
    https://git.kernel.org/netdev/net-next/c/2ab399a931dd
  - [net-next,6/6] sctp: delete the nested flexible array payload
    https://git.kernel.org/netdev/net-next/c/dbda0fba7a14

You are awesome, thank you!
Kees Cook Dec. 14, 2023, 4:55 p.m. UTC | #3
On Wed, Apr 19, 2023 at 06:18:24PM -0700, Jakub Kicinski wrote:
> On Wed, 19 Apr 2023 11:16:27 -0400 Xin Long wrote:
> > Paolo noticed a compile warning in SCTP,
> > 
> > ../net/sctp/stream_sched_fc.c: note: in included file (through ../include/net/sctp/sctp.h):
> > ../include/net/sctp/structs.h:335:41: warning: array of flexible structures
> > 
> > But not only this, there are actually quite a lot of such warnings in
> > some SCTP structs. This patchset fixes most of warnings by deleting
> > these nested flexible array members.
> > 
> > After this patchset, there are still some warnings left:
> > 
> >   # make C=2 CF="-Wflexible-array-nested" M=./net/sctp/
> >   ./include/net/sctp/structs.h:1145:41: warning: nested flexible array
> >   ./include/uapi/linux/sctp.h:641:34: warning: nested flexible array
> >   ./include/uapi/linux/sctp.h:643:34: warning: nested flexible array
> >   ./include/uapi/linux/sctp.h:644:33: warning: nested flexible array
> >   ./include/uapi/linux/sctp.h:650:40: warning: nested flexible array
> >   ./include/uapi/linux/sctp.h:653:39: warning: nested flexible array
> > 
> > the 1st is caused by __data[] in struct ip_options, not in SCTP;
> > the others are in uapi, and we should not touch them.
> > 
> > Note that instead of completely deleting it, we just leave it as a
> > comment in the struct, signalling to the reader that we do expect
> > such variable parameters over there, as Marcelo suggested.
> 
> Hi Kees, is there no workaround for nested flexible arrays within 
> the kernel?  Any recommendations?
> 
> https://lore.kernel.org/all/cover.1681917361.git.lucien.xin@gmail.com/

*thread necromancy*

Hi, I apologize for missing this thread back in April!

There's no need for a work-around: this situation isn't a problem.
Composite structures that end with a flexible array are perfectly valid
(the compiler can unambiguously reason about sizes). So none of these
patches are needed (and actually reduce the compiler's ability to reason
about object sizes). We shouldn't run sparse with -Wflexible-array-nested
as this isn't an actual problem.

The only problem that can happen like this is when a flex array
ends up in the _middle_ of a composite structure, in which
case yes, this needs to be fixed. This check is supported by
GCC 14+ with -Wflex-array-member-not-at-end. For example, see:
https://lore.kernel.org/all/1da736106d8e0806aeafa6e471a13ced490eae22.1698117815.git.gustavoars@kernel.org/

-Kees