Daniel Borkmann wrote:
> On 11/7/20 8:37 PM, John Fastabend wrote:
> > If copy_page_to_iter() fails or even partially completes, but with fewer
> > bytes copied than expected we currently reset sg.start and return EFAULT.
> > This proves problematic if we already copied data into the user buffer
> > before we return an error. Because we leave the copied data in the user
> > buffer and fail to unwind the scatterlist so kernel side believes data
> > has been copied and user side believes data has _not_ been received.

[...]

> > +                   if (!copy) {
> > +                           return copied ? copied : -EFAULT;
> >                     }
> 
> nit: no need for {}
> 
> >   
> >                     copied += copy;
> > @@ -56,6 +55,11 @@ int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock 
> > *psock,
> >                                             put_page(page);
> >                             }
> >                     } else {
> > +                           /* Lets not optimize peek case if 
> > copy_page_to_iter
> > +                            * didn't copy the entire length lets just 
> > break.
> > +                            */
> > +                           if (copy != sge->length)
> > +                                   goto out;
> 
> nit: return copied;
> 
> Rest lgtm for this one.

Great, thanks for the review will fixup in v2.

Reply via email to