RE: Submitting fixes for bugs: Re: tun device patch for IPv6
From: Zabele, Stephen \(US SSA\)
Date: Wed Jun 04 2008 - 08:50:50 EST
Thanks Max, much appreciated.
>From what I understand, the copy_from_user is needed to protect kernels
that are running on multiprocessor hosts -- on some other work we were
doing a while back, we had left the copy out and were just using the
provided pointer directly to access information in the buffer -- and it
was causing intermittent kernel panics.
Steve
-----Original Message-----
From: Max Krasnyanskiy [mailto:maxk@xxxxxxxxxxxx]
Sent: Tuesday, June 03, 2008 2:56 PM
To: Zabele, Stephen (US SSA)
Cc: Natalie Protasevich; Andrew Morton; LKML
Subject: Re: Submitting fixes for bugs: Re: tun device patch for IPv6
Zabele, Stephen (US SSA) wrote:
> Max,
>
> I am confused by your comment. As you point out, the TUN driver does
> have an option that allows users to provide the protocol information
as
> an extended header. The problem is that there is a lot of existing
IPv4
> code that does not use this approach, and we'd like to make it easy
for
> folsk to port their existing code to work with IPv6.
>
> Hence, the patch provided below allows existing software that does not
> use the extra protocol information to work with IPv6.
>
> If you are suggesting that "people shouldn't be using the non-PI
option"
> for the TUN device, then the option should be taken out to force the
> issue rather than leaving an incomplete implementations lying around
the
> kernel and making folks spend a lot of hours trying to debug why the
> non-PI version doesn't work with IPv6 -- like I did.
>
> I disagree with this assertion, however, because providing this
> information forces a lot of extra (and in my opinion unnecessary)
coding
> in user space.
I guess the argument makes sense. And it's been submitted a few times
already.
So if people insist :). The patch is not the best way to do it though. I
do
not think we need an extra copy_from_user(). I'll either dig out an
original
patch or cook up a new one and then push it upstream along a couple of
other
TUN bug fixes.
Max
>
> Steve
>
> -----Original Message-----
> From: Max Krasnyansky [mailto:maxk@xxxxxxxxxxxx]
> Sent: Monday, June 02, 2008 11:36 PM
> To: Natalie Protasevich
> Cc: Zabele, Stephen (US SSA); Andrew Morton; LKML
> Subject: Re: Submitting fixes for bugs: Re: tun device patch for IPv6
>
> btw I naked similar patch a couple of times already.
> TUN driver provide PI (protocol info) extension that lets uses set
> packet
> type. Please use that instead.
>
> Max
>
> Natalie Protasevich wrote:
>> On Mon, Jun 2, 2008 at 6:53 AM, Zabele, Stephen (US SSA)
>> <steve.zabele@xxxxxxxxxxxxxx> wrote:
>>> Natalie,
>>>
>>>> Steve,
>>>> Can you please submit the patch to LKML and to Maxim
>>> (maxk@xxxxxxxxxxxx) >via email (if you haven't already)?
>>>
>>> The patch is accessible from the bottom of the bugzilla web page,
but
>>> I'm also providing it again here for reference. I' not sure how to
>>> submit it to LKML other than through the bugzilla mechanism. Can you
>>> provide me a pointer??
>>>
>>> Thanks!
>>>
>>> Steve
>>>
>> Hi Steve,
>> This issue is pretty generic. Bug reporters often produce fixes, that
>> remain in bugzilla not being harvested, simply because bugzilla is
not
>> designed for submitting patches. Even developers who work on bugzilla
>> and leave their patches attached to the bug don't get those
>> incorporated until they submit it conventional way.
>>
>> To submit the patch properly follow
>> http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt. There are
>> other pointers like http://linux.yyz.us/patch-format.html,
>> http://lxr.linux.no/linux/Documentation/SubmittingPatches. You can
>> look at any submitted patch as a sample and examine the format; note
>> that added/changed/removed lines produced with diffstat.
>>
>> Maintainers deal with great number of patches, so in order to be able
>> to use your patch they have go receive it in proper format i.e. with
>> conventional comments and headers, such that it complies with source
>> keeping process (not mentioning that it should apply to latest tree,
>> so they don't have to port it...)
>> Any patch that fixes a problem is valuable, so by submitting it
proper
>> way you'll make sure (1) your patch will get the best review possible
>> (2) you will get proper credit for it. Then you post it to LKML and
>> CC to subsystem maintainer and/or developers heavily involved with
>> this module.
>> Thanks,
>> --Natalie
>>
>>> --- Cut here ---
>>>
>>> --- a/drivers/net/tun.c 2008-02-11 01:06:32.000000000 -0500
>>> +++ b/drivers/net/tun.c 2008-03-18 15:30:55.000000000 -0400
>>> @@ -243,8 +243,26 @@
>>>
>>> if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
>>> return -EFAULT;
>>> - }
>>> -
>>> +
>>> + } else if ((tun->flags & TUN_TYPE_MASK) == TUN_TUN_DEV) {
>>> + if (iv->iov_len) {
>>> + unsigned char verbuf;
>>> +
>>> + if (copy_from_user((void *)&verbuf,
>>> iv->iov_base, sizeof(verbuf)))
>>> + return -EFAULT;
>>> +
>>> + switch (verbuf & 0xf0) {
>>> + case 0x40:
>>> + break;
>>> + case 0x60:
>>> + pi.proto = htons(ETH_P_IPV6);
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> + }
>>> + }
>>> +
>>> if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV)
>>> align = NET_IP_ALIGN;
>>>
>>>
>> --
>> 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/