From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: Re: [PATCH] sctp: fix a potential missing-check bug Date: Thu, 3 May 2018 09:46:29 -0300 Message-ID: <20180503124629.GM5105@localhost.localdomain> References: <1525310145-28102-1-git-send-email-wang6495@umn.edu> <20180503012402.GK5105@localhost.localdomain> <20180503014838.GL5105@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Kangjie Lu , Vlad Yasevich , Neil Horman , "David S. Miller" , "open list:SCTP PROTOCOL" , "open list:NETWORKING [GENERAL]" , open list To: Wenwen Wang Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Thu, May 03, 2018 at 07:01:51AM -0500, Wenwen Wang wrote: > On Wed, May 2, 2018 at 8:48 PM, Marcelo Ricardo Leitner > wrote: > > On Wed, May 02, 2018 at 08:27:05PM -0500, Wenwen Wang wrote: > >> On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner > >> wrote: > >> > On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote: > >> >> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len > >> >> and max_len to check whether it is in the appropriate range. If it is not, > >> >> an error code -EINVAL will be returned. This is enforced by a security > >> >> check. But, this check is only executed when 'val' is not 0. In fact, if > >> >> 'val' is 0, it will be assigned with a new value (if the return value of > >> >> the function sctp_id2assoc() is not 0) in the following execution. However, > >> >> this new value of 'val' is not checked before it is used to assigned to > >> >> asoc->user_frag. That means it is possible that the new value of 'val' > >> >> could be out of the expected range. This can cause security issues > >> >> such as buffer overflows, e.g., the new value of 'val' is used as an index > >> >> to access a buffer. > >> >> > >> >> This patch inserts a check for the new value of 'val' to see if it is in > >> >> the expected range. If it is not, an error code -EINVAL will be returned. > >> >> > >> >> Signed-off-by: Wenwen Wang > >> >> --- > >> >> net/sctp/socket.c | 22 +++++++++++----------- > >> >> 1 file changed, 11 insertions(+), 11 deletions(-) > >> > > >> > ? > >> > This patch is the same as previous one. git send-email > >> > maybe? > >> > > >> > Marcelo > >> > >> Thanks for your suggestion, Marcelo. I can send the old file. But, I > >> have added a line of comment in this patch. > > > > I meant if you had sent the old patch again by accident, because you > > said you worked on an old version of the tree, but then posted a patch > > that also doesn't use the new MTU function I mentioned. > > > > Marcelo > > I worked on the latest kernel. But, I didn't find the MTU function > sctp_mtu_payload(). Which tree are you using? [a] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git or [b] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git ? The function isn't on [a] yet, but it is on [b]. Marcelo