Re: [PATCH 3/5] tmem: Check copy_to_user_* return value.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 26, 2013 at 08:21:09AM +0000, Jan Beulich wrote:
> >>> On 25.11.13 at 18:00, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> 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.

<nods>
> 
> Jan
>