From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 3/5] tmem: Check copy_to_user_* return value. Date: Tue, 26 Nov 2013 13:16:22 -0500 Message-ID: <20131126181622.GB2838@phenom.dumpdata.com> References: <1385398842-8247-1-git-send-email-konrad.wilk@oracle.com> <1385398842-8247-4-git-send-email-konrad.wilk@oracle.com> <529468050200007800106FA0@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VlNBV-00015c-LW for xen-devel@lists.xenproject.org; Tue, 26 Nov 2013 18:16:29 +0000 Content-Disposition: inline In-Reply-To: <529468050200007800106FA0@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Tue, Nov 26, 2013 at 08:21:09AM +0000, Jan Beulich wrote: > >>> On 25.11.13 at 18:00, Konrad Rzeszutek Wilk wrote: > > First of all, the title is wrong: You really mean copy_to_guest*(). > > > We weren't checking whether that operation fails and > > return the proper error. > > > > This fixes CID 1055125, 105512, 1055127, 1055128, 1055129, > > 1055130. > > But if you're doing something like this, you should fix all instances, > not just some. Which would e.g. require > tmem_copy_to_client_buf_offset() to propagate the return value > of copy_to_guest_offset() (it's odd anyway that this one is an > inline function, while tmem_copy_to_client_buf() is a macro). No need. One of the Bobs patches fixes that. (the ones he posted a couple of days ago that - not the ones you pulled). > > But then again I'm wondering what baseline your patch uses: It was on top of the earlier tmem patches. Which is staging, so this should apply nicely on top of that. But I also realize that it has the first of his patches that he had posted this week. That is the tmem: cleanup: drop some debug code patch. Sorry about that - I should have mentioned that in the cover letter and I completely forgot. > > > --- a/xen/common/tmem.c > > +++ b/xen/common/tmem.c > > @@ -2146,8 +2146,12 @@ static int tmemc_list(domid_t cli_id, tmem_cli_va_param_t buf, uint32_t len, > > if ( cli_id == TMEM_CLI_ID_NULL ) { > > off = tmemc_list_global(buf,0,len,use_long); > > off += tmemc_list_shared(buf,off,len-off,use_long); > > This isn't on line 2146 of today's staging tree, but on line 2239. > > Hence looking at the individual changes may not make much sense, > as it's not clear whether there are other dependencies on earlier > changes here. > > Jan >