From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 24 Aug 2018 16:55:00 +0300 From: Vladimir Davydov Subject: Re: [PATCH 3/3] socket: prevent recvfrom from returning garbage Message-ID: <20180824135500.pplinofqzzetwutd@esperanza> References: <0c53f7379963bb5fb61c97e08d782357bd82f562.1535076888.git.alexander.turenko@tarantool.org> <20180824130748.duye24rcblum5f4z@esperanza> <20180824134441.nlb3fe2crb6dljoo@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180824134441.nlb3fe2crb6dljoo@tkn_work_nb> To: Alexander Turenko Cc: tarantool-patches@freelists.org List-ID: On Fri, Aug 24, 2018 at 04:44:41PM +0300, Alexander Turenko wrote: > On Fri, Aug 24, 2018 at 04:07:48PM +0300, Vladimir Davydov wrote: > > On Fri, Aug 24, 2018 at 05:47:39AM +0300, Alexander Turenko wrote: > > > In C recvfrom function sets addrlen parameter to zero when called on TCP > > > socket (at least on Linux). The src_addr parameter can contain garbage > > > in the case, so we should not dereference it. > > > > > > Before this commit socket:recvfrom() can return 'from' table with only > > > family field (don't sure why, but addr->sa_family often contain PF_INET > > > value in my case) or return nil depending on the garbage at the address. > > > Now it always return nil. > > > --- > > > src/lua/socket.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > Could you please add a test case? > > Added in the previous commit ('case: recvfrom truncate the message with > tcp'). The main purpose of the test case is another, but when added I > found the flakiness ('from' could be {family = 'AF_INET'} or nil from > time to time) and fixed it here. > > How to better handle this? I suppose I should add a comment to the test > case that it also test this case? This patch is pretty straightforward and raises no questions, in contrast to patches 1 and 2. If you equipped it with a test and rebased on top of 1.10 (or should it be 1.9?), I'd pushed it right away.