Re: [PATCH net] tun: handle copy failure in tun_put_user()

[Michael S. Tsirkin]

On Mon, Jan 20, 2014 at 05:32:02PM +0800, Jason Wang wrote:
> On 01/20/2014 04:43 PM, Michael S. Tsirkin wrote:
> > On Sun, Jan 19, 2014 at 07:48:56PM -0800, David Miller wrote:
> >> From: Jason Wang <jasowang@xxxxxxxxxx>
> >> Date: Mon, 20 Jan 2014 11:16:48 +0800
> >>
> >>> This patch return the error code of copy helpers in tun_put_user() instead of
> >>> ignoring them.
> >>>
> >>> Cc: Michael S. Tsirkin <mst@xxxxxxxxxx>
> >>> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> > I'm not sure we need to worry about this too much.
> > But if yes, a bunch of places besides tun should be
> > changed.
> 
> Yes, I send the patch because the error processing here is different
> from what macvtap does. Macvtap just return error in this case and so do
> packet socket.

I suspect we just need to document that invalid address simply results
in unspecified behaviour.  We try to return EFAULT to help debugging
sometimes but it's on a best effort basis.
>From this point of view EFAULT seems easier to debug than truncating the packet.
In any case even if we change Linux - applications won't be able to rely
on this for a long while.
So maybe we shouldn't do anything.


> >  Consider for example udp_recvmsg: it
> > never seems to return any error except -EAGAIN.
> >
> > Is this a bug? Man page for recvmsg says:
> >      EFAULT The receive buffer pointer(s)  point  outside  the process's  address
> >               space.
> >
> > this isn't very clear: does this mean "all pointers are invalid"
> > or "some pointers are invalid"?
> > Also, what if pointers themselves are valid but length
> > makes us go outside the address space?
> >
> > I'm guessing the simplest way is to clarify in the man page that
> > passing invalid pointers / lengths is not guaranteed
> > to result in EFAULT and that Linux makes no guarantees
> > about the returned length in this case.
> >
> > Cc linux-man in case they can suggest some insights on this.
> >
> >> If you perform some of the copy successfully, you have to report that
> >> length rather than just an error.
> >>
> >> Otherwise userland has no way to determine how much of the data was
> >> successfully sourced.
> >>
> >> I'm not applying this, sorry.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/