From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vladislav Yasevich Date: Mon, 02 Apr 2012 13:49:33 +0000 Subject: Re: question about net/sctp/socket.c Message-Id: <4F79AE6D.8060203@hp.com> List-Id: References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sctp@vger.kernel.org On 04/02/2012 09:24 AM, Julia Lawall wrote: > In the file net/sctp/socket.c, the function sctp_getsockopt_peeloff ends with: > > retval = sock_map_fd(newsock, 0); > if (retval < 0) { > sock_release(newsock); > goto out; > } > > SCTP_DEBUG_PRINTK("%s: sk: %p newsk: %p sd: %d\n", > __func__, sk, newsock->sk, retval); > > /* Return the fd mapped to the new socket. */ > peeloff.sd = retval; > if (put_user(len, optlen)) > return -EFAULT; > if (copy_to_user(optval, &peeloff, len)) > retval = -EFAULT; > > Should there be a call to sock_release in the final two error cases as well? I don't see anything that removes the need for it. And is some cleanup of the effect of sock_map_fd needed as well? > > thanks, > julia > Hi Julia This is an interesting case. The possible issue I see calling sock_release here as well as after sock_map_fd call is that if the peeloff call fails in these 3 instances, the association is terminated. There is no graceful recovery at all. It may make sense to re-arrange the code so that failures here are recoverable. By that I mean, that if a call to peeloff() fails, the original socket and association lists aren't touched. That would allow a repeated call to peeloff to potentially succeed. -vlad