From: Neil Horman <nhorman@tuxdriver.com> To: Xin Long <lucien.xin@gmail.com> Cc: network dev <netdev@vger.kernel.org>, linux-sctp@vger.kernel.org, davem@davemloft.net, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Subject: Re: [PATCH net 1/3] sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails Date: Wed, 6 Mar 2019 13:21:27 -0500 [thread overview] Message-ID: <20190306182127.GB10683@neilslaptop.think-freely.org> (raw) In-Reply-To: <6837e72485125c8740900fd17fa84ac68b8892a5.1551606805.git.lucien.xin@gmail.com> On Sun, Mar 03, 2019 at 05:54:53PM +0800, Xin Long wrote: > It should fail to create the new sk if sctp_bind_addr_dup() fails > when accepting or peeloff an association. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/sctp/socket.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index a2771b3..22adb8d 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -102,9 +102,9 @@ static int sctp_send_asconf(struct sctp_association *asoc, > struct sctp_chunk *chunk); > static int sctp_do_bind(struct sock *, union sctp_addr *, int); > static int sctp_autobind(struct sock *sk); > -static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > - struct sctp_association *assoc, > - enum sctp_socket_type type); > +static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > + struct sctp_association *assoc, > + enum sctp_socket_type type); > > static unsigned long sctp_memory_pressure; > static atomic_long_t sctp_memory_allocated; > @@ -4655,7 +4655,11 @@ static struct sock *sctp_accept(struct sock *sk, int flags, int *err, bool kern) > /* Populate the fields of the newsk from the oldsk and migrate the > * asoc to the newsk. > */ > - sctp_sock_migrate(sk, newsk, asoc, SCTP_SOCKET_TCP); > + error = sctp_sock_migrate(sk, newsk, asoc, SCTP_SOCKET_TCP); > + if (error) { > + sk_common_release(newsk); sctp_sock_migrate may fail after the pending packets have been moved from the old socket to the new socket. Normally those packets will get purged by successful transmission, or when the socket is closed (via sctp_close), but neither of those cases applies here. Whats going to dequeue and free any pending skbs on the sk_receive_queue here? > + newsk = NULL; > + } > > out: > release_sock(sk); > @@ -5401,7 +5405,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) > /* Populate the fields of the newsk from the oldsk and migrate the > * asoc to the newsk. > */ > - sctp_sock_migrate(sk, sock->sk, asoc, SCTP_SOCKET_UDP_HIGH_BANDWIDTH); > + err = sctp_sock_migrate(sk, sock->sk, asoc, > + SCTP_SOCKET_UDP_HIGH_BANDWIDTH); > + if (err) { > + sock_release(sock); Same question here, what frees any pending skbs on the new socket, if the migration fails after the skbs have been queued to it? > + sock = NULL; > + } > > *sockp = sock; > > @@ -8924,9 +8933,9 @@ static inline void sctp_copy_descendant(struct sock *sk_to, > /* Populate the fields of the newsk from the oldsk and migrate the assoc > * and its messages to the newsk. > */ > -static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > - struct sctp_association *assoc, > - enum sctp_socket_type type) > +static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > + struct sctp_association *assoc, > + enum sctp_socket_type type) > { > struct sctp_sock *oldsp = sctp_sk(oldsk); > struct sctp_sock *newsp = sctp_sk(newsk); > @@ -8935,6 +8944,7 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > struct sk_buff *skb, *tmp; > struct sctp_ulpevent *event; > struct sctp_bind_hashbucket *head; > + int err; > > /* Migrate socket buffer sizes and all the socket level options to the > * new socket. > @@ -8963,8 +8973,10 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > /* Copy the bind_addr list from the original endpoint to the new > * endpoint so that we can handle restarts properly > */ > - sctp_bind_addr_dup(&newsp->ep->base.bind_addr, > - &oldsp->ep->base.bind_addr, GFP_KERNEL); > + err = sctp_bind_addr_dup(&newsp->ep->base.bind_addr, > + &oldsp->ep->base.bind_addr, GFP_KERNEL); > + if (err) > + return err; > > /* Move any messages in the old socket's receive queue that are for the > * peeled off association to the new socket's receive queue. > @@ -9049,6 +9061,8 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > } > > release_sock(newsk); > + > + return 0; > } > > > -- > 2.1.0 > >
WARNING: multiple messages have this Message-ID (diff)
From: Neil Horman <nhorman@tuxdriver.com> To: Xin Long <lucien.xin@gmail.com> Cc: network dev <netdev@vger.kernel.org>, linux-sctp@vger.kernel.org, davem@davemloft.net, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Subject: Re: [PATCH net 1/3] sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails Date: Wed, 06 Mar 2019 18:21:27 +0000 [thread overview] Message-ID: <20190306182127.GB10683@neilslaptop.think-freely.org> (raw) In-Reply-To: <6837e72485125c8740900fd17fa84ac68b8892a5.1551606805.git.lucien.xin@gmail.com> On Sun, Mar 03, 2019 at 05:54:53PM +0800, Xin Long wrote: > It should fail to create the new sk if sctp_bind_addr_dup() fails > when accepting or peeloff an association. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/sctp/socket.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index a2771b3..22adb8d 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -102,9 +102,9 @@ static int sctp_send_asconf(struct sctp_association *asoc, > struct sctp_chunk *chunk); > static int sctp_do_bind(struct sock *, union sctp_addr *, int); > static int sctp_autobind(struct sock *sk); > -static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > - struct sctp_association *assoc, > - enum sctp_socket_type type); > +static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > + struct sctp_association *assoc, > + enum sctp_socket_type type); > > static unsigned long sctp_memory_pressure; > static atomic_long_t sctp_memory_allocated; > @@ -4655,7 +4655,11 @@ static struct sock *sctp_accept(struct sock *sk, int flags, int *err, bool kern) > /* Populate the fields of the newsk from the oldsk and migrate the > * asoc to the newsk. > */ > - sctp_sock_migrate(sk, newsk, asoc, SCTP_SOCKET_TCP); > + error = sctp_sock_migrate(sk, newsk, asoc, SCTP_SOCKET_TCP); > + if (error) { > + sk_common_release(newsk); sctp_sock_migrate may fail after the pending packets have been moved from the old socket to the new socket. Normally those packets will get purged by successful transmission, or when the socket is closed (via sctp_close), but neither of those cases applies here. Whats going to dequeue and free any pending skbs on the sk_receive_queue here? > + newsk = NULL; > + } > > out: > release_sock(sk); > @@ -5401,7 +5405,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp) > /* Populate the fields of the newsk from the oldsk and migrate the > * asoc to the newsk. > */ > - sctp_sock_migrate(sk, sock->sk, asoc, SCTP_SOCKET_UDP_HIGH_BANDWIDTH); > + err = sctp_sock_migrate(sk, sock->sk, asoc, > + SCTP_SOCKET_UDP_HIGH_BANDWIDTH); > + if (err) { > + sock_release(sock); Same question here, what frees any pending skbs on the new socket, if the migration fails after the skbs have been queued to it? > + sock = NULL; > + } > > *sockp = sock; > > @@ -8924,9 +8933,9 @@ static inline void sctp_copy_descendant(struct sock *sk_to, > /* Populate the fields of the newsk from the oldsk and migrate the assoc > * and its messages to the newsk. > */ > -static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > - struct sctp_association *assoc, > - enum sctp_socket_type type) > +static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > + struct sctp_association *assoc, > + enum sctp_socket_type type) > { > struct sctp_sock *oldsp = sctp_sk(oldsk); > struct sctp_sock *newsp = sctp_sk(newsk); > @@ -8935,6 +8944,7 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > struct sk_buff *skb, *tmp; > struct sctp_ulpevent *event; > struct sctp_bind_hashbucket *head; > + int err; > > /* Migrate socket buffer sizes and all the socket level options to the > * new socket. > @@ -8963,8 +8973,10 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > /* Copy the bind_addr list from the original endpoint to the new > * endpoint so that we can handle restarts properly > */ > - sctp_bind_addr_dup(&newsp->ep->base.bind_addr, > - &oldsp->ep->base.bind_addr, GFP_KERNEL); > + err = sctp_bind_addr_dup(&newsp->ep->base.bind_addr, > + &oldsp->ep->base.bind_addr, GFP_KERNEL); > + if (err) > + return err; > > /* Move any messages in the old socket's receive queue that are for the > * peeled off association to the new socket's receive queue. > @@ -9049,6 +9061,8 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, > } > > release_sock(newsk); > + > + return 0; > } > > > -- > 2.1.0 > >
next prev parent reply other threads:[~2019-03-06 18:21 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-03 9:54 [PATCH net 0/3] sctp: process the error returned from sctp_sock_migrate() Xin Long 2019-03-03 9:54 ` Xin Long 2019-03-03 9:54 ` [PATCH net 1/3] sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails Xin Long 2019-03-03 9:54 ` Xin Long 2019-03-03 9:54 ` [PATCH net 2/3] sctp: move up sctp_auth_init_hmacs() in sctp_endpoint_init() Xin Long 2019-03-03 9:54 ` Xin Long 2019-03-03 9:54 ` [PATCH net 3/3] sctp: call sctp_auth_init_hmacs() in sctp_sock_migrate() Xin Long 2019-03-03 9:54 ` Xin Long 2019-03-06 18:26 ` Neil Horman 2019-03-06 18:26 ` Neil Horman 2019-03-06 18:24 ` [PATCH net 2/3] sctp: move up sctp_auth_init_hmacs() in sctp_endpoint_init() Neil Horman 2019-03-06 18:24 ` Neil Horman 2019-03-06 18:21 ` Neil Horman [this message] 2019-03-06 18:21 ` [PATCH net 1/3] sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails Neil Horman 2019-03-07 10:06 ` Xin Long 2019-03-07 10:06 ` Xin Long 2019-03-07 11:59 ` Neil Horman 2019-03-07 11:59 ` Neil Horman 2019-03-07 18:25 ` Marcelo Ricardo Leitner 2019-03-07 18:25 ` Marcelo Ricardo Leitner 2019-03-08 3:48 ` Xin Long 2019-03-08 3:48 ` Xin Long 2019-03-08 16:59 ` Marcelo Ricardo Leitner 2019-03-08 16:59 ` Marcelo Ricardo Leitner 2019-03-04 19:04 ` [PATCH net 0/3] sctp: process the error returned from sctp_sock_migrate() David Miller 2019-03-04 19:04 ` David Miller 2019-03-07 14:59 ` Marcelo Ricardo Leitner 2019-03-07 14:59 ` Marcelo Ricardo Leitner 2019-03-07 12:11 ` Neil Horman 2019-03-07 12:11 ` Neil Horman 2019-03-08 17:00 ` Marcelo Ricardo Leitner 2019-03-08 17:00 ` Marcelo Ricardo Leitner 2019-03-08 19:43 ` David Miller 2019-03-08 19:43 ` David Miller
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190306182127.GB10683@neilslaptop.think-freely.org \ --to=nhorman@tuxdriver.com \ --cc=davem@davemloft.net \ --cc=linux-sctp@vger.kernel.org \ --cc=lucien.xin@gmail.com \ --cc=marcelo.leitner@gmail.com \ --cc=netdev@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.