RE: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds

[David Laight <David.Laight@ACULAB.COM>]

From: Xin Long [mailto:lucien.xin@gmail.com]
> Sent: 11 September 2019 09:52
> On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long
> > > Sent: 09 September 2019 08:57
> > > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> > >
> > > Note that ps_retrans is not allowed to be greater than pf_retrans.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  include/uapi/linux/sctp.h |  1 +
> > >  net/sctp/socket.c         | 10 ++++++++++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > index a15cc28..dfd81e1 100644
> > > --- a/include/uapi/linux/sctp.h
> > > +++ b/include/uapi/linux/sctp.h
> > > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> > >       struct sockaddr_storage spt_address;
> > >       __u16 spt_pathmaxrxt;
> > >       __u16 spt_pathpfthld;
> > > +     __u16 spt_pathcpthld;
> > >  };
> > >
> > >  /*
> > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > index 5e2098b..5b9774d 100644
> > > --- a/net/sctp/socket.c
> > > +++ b/net/sctp/socket.c
> > > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
> >
> > This code does:
> >         if (optlen < sizeof(struct sctp_paddrthlds))
> >                 return -EINVAL;
> here will become:
> 
>         if (optlen >= sizeof(struct sctp_paddrthlds)) {
>                 optlen = sizeof(struct sctp_paddrthlds);
>         } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
>                                             spt_pathcpthld), 4))
>                 optlen = ALIGN(offsetof(struct sctp_paddrthlds,
>                                         spt_pathcpthld), 4);
>                 val.spt_pathcpthld = 0xffff;
>         else {
>                 return -EINVAL;
>         }

Hmmm...
If the kernel has to default 'val.spt_pathcpthld = 0xffff'
then recompiling an existing application with the new uapi
header is going to lead to very unexpected behaviour.

The best you can hope for is that the application memset the
structure to zero.
But more likely it is 'random' on-stack data.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)