Re: [PATCH net-next 1/4] sctp: check addr_size with sa_family_t size in __sctp_setsockopt_connectx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 23, 2019 at 11:25 PM Neil Horman <nhorman@xxxxxxxxxxxxx> wrote:
>
> On Tue, Jul 23, 2019 at 01:37:57AM +0800, Xin Long wrote:
> > Now __sctp_connect() is called by __sctp_setsockopt_connectx() and
> > sctp_inet_connect(), the latter has done addr_size check with size
> > of sa_family_t.
> >
> > In the next patch to clean up __sctp_connect(), we will remove
> > addr_size check with size of sa_family_t from __sctp_connect()
> > for the 1st address.
> >
> > So before doing that, __sctp_setsockopt_connectx() should do
> > this check first, as sctp_inet_connect() does.
> >
> > Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx>
> > ---
> >  net/sctp/socket.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index aa80cda..5f92e4a 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -1311,7 +1311,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
> >       pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
> >                __func__, sk, addrs, addrs_size);
> >
> > -     if (unlikely(addrs_size <= 0))
> > +     if (unlikely(addrs_size < sizeof(sa_family_t)))
> I don't think this is what you want to check for here.  sa_family_t is
> an unsigned short, and addrs_size is the number of bytes in the addrs
> array.  The addrs array should be at least the size of one struct
> sockaddr (16 bytes iirc), and, if larger, should be a multiple of
> sizeof(struct sockaddr)
sizeof(struct sockaddr) is not the right value to check either.

The proper check will be done later in __sctp_connect():

        af = sctp_get_af_specific(daddr->sa.sa_family);
        if (!af || af->sockaddr_len > addrs_size)
                return -EINVAL;

So the check 'addrs_size < sizeof(sa_family_t)' in this patch is
just to make sure daddr->sa.sa_family is accessible. the same
check is also done in sctp_inet_connect().

>
> Neil
>
> >               return -EINVAL;
> >
> >       kaddrs = memdup_user(addrs, addrs_size);
> > --
> > 2.1.0
> >
> >



[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux