Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/11/2013 01:12 PM, David Miller wrote: > From: Jason Wang > Date: Fri, 11 Jan 2013 09:29:20 +0800 > >> On 01/11/2013 06:39 AM, David Miller wrote: >>> From: Stefan Hajnoczi >>> Date: Thu, 10 Jan 2013 08:59:48 +0100 >>> Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface "B" after detaching from interface "A". The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi >>> Applied. >> Hi David: >> >> Any chance that I can have a respin on this patch, there's still a bug >> after this patch. Or I just can send a patch on top? > If I've applied it, there is no reverting. Get it, will send patch on top. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/11/2013 01:12 PM, David Miller wrote: From: Jason Wang jasow...@redhat.com Date: Fri, 11 Jan 2013 09:29:20 +0800 On 01/11/2013 06:39 AM, David Miller wrote: From: Stefan Hajnoczi stefa...@redhat.com Date: Thu, 10 Jan 2013 08:59:48 +0100 Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Applied. Hi David: Any chance that I can have a respin on this patch, there's still a bug after this patch. Or I just can send a patch on top? If I've applied it, there is no reverting. Get it, will send patch on top. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
From: Jason Wang Date: Fri, 11 Jan 2013 09:29:20 +0800 > On 01/11/2013 06:39 AM, David Miller wrote: >> From: Stefan Hajnoczi >> Date: Thu, 10 Jan 2013 08:59:48 +0100 >> >>> Multiqueue tun devices support detaching a tun_file from its tun_struct >>> and re-attaching at a later point in time. This allows users to disable >>> a specific queue temporarily. >>> >>> ioctl(TUNSETIFF) allows the user to specify the network interface to >>> attach by name. This means the user can attempt to attach to interface >>> "B" after detaching from interface "A". >>> >>> The driver is not designed to support this so check we are re-attaching >>> to the right tun_struct. Failure to do so may lead to oops. >>> >>> Signed-off-by: Stefan Hajnoczi >> Applied. > Hi David: > > Any chance that I can have a respin on this patch, there's still a bug > after this patch. Or I just can send a patch on top? If I've applied it, there is no reverting. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/11/2013 06:39 AM, David Miller wrote: > From: Stefan Hajnoczi > Date: Thu, 10 Jan 2013 08:59:48 +0100 > >> Multiqueue tun devices support detaching a tun_file from its tun_struct >> and re-attaching at a later point in time. This allows users to disable >> a specific queue temporarily. >> >> ioctl(TUNSETIFF) allows the user to specify the network interface to >> attach by name. This means the user can attempt to attach to interface >> "B" after detaching from interface "A". >> >> The driver is not designed to support this so check we are re-attaching >> to the right tun_struct. Failure to do so may lead to oops. >> >> Signed-off-by: Stefan Hajnoczi > Applied. Hi David: Any chance that I can have a respin on this patch, there's still a bug after this patch. Or I just can send a patch on top? Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
From: Stefan Hajnoczi Date: Thu, 10 Jan 2013 08:59:48 +0100 > Multiqueue tun devices support detaching a tun_file from its tun_struct > and re-attaching at a later point in time. This allows users to disable > a specific queue temporarily. > > ioctl(TUNSETIFF) allows the user to specify the network interface to > attach by name. This means the user can attempt to attach to interface > "B" after detaching from interface "A". > > The driver is not designed to support this so check we are re-attaching > to the right tun_struct. Failure to do so may lead to oops. > > Signed-off-by: Stefan Hajnoczi Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/10/2013 07:07 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 10, 2013 at 06:43:20PM +0800, Jason Wang wrote:
>> On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
>>> On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
Multiqueue tun devices support detaching a tun_file from its tun_struct
and re-attaching at a later point in time. This allows users to disable
a specific queue temporarily.
ioctl(TUNSETIFF) allows the user to specify the network interface to
attach by name. This means the user can attempt to attach to interface
"B" after detaching from interface "A".
The driver is not designed to support this so check we are re-attaching
to the right tun_struct. Failure to do so may lead to oops.
Signed-off-by: Stefan Hajnoczi
---
This fix is for 3.8-rc.
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..cf6da6e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
err = -EINVAL;
if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
goto out;
+ if (tfile->detached && tun != tfile->detached)
+ goto out;
err = -EBUSY;
if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
--
1.8.0.2
>>> I agree this is a bug but even with this patch, we still allow:
>>>
>>> SETIFF
>>> SETQUEUE (DISABLED)
>>> SETIFF
>>>
>>> Originally the rule always was that repeated setiff calls fail with
>>> EINVAL. We broke that when we set tun to NULL. It's probably worth
>>> preserving that, even if queue is disabled. Applying something like the
>>> below
>>> instead will address this concern, won't it?
>>>
>>> Note: works with regular userspace but I didn't test
>>> multiqueue userspace. What do you think.
>> Or just fail when tun->detached is not NULL at the beginning of
>> tun_set_iff()?
> Yes that'll also work I think. If you prefer this pls send patch.
Will post a patch, thanks.
>>> Signed-off-by: Michael S. Tsirkin
>>>
>>> ---
>>>
>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>> index fbd106e..5ec8b08 100644
>>> --- a/drivers/net/tun.c
>>> +++ b/drivers/net/tun.c
>>> @@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
>>> BUG_ON(tun->numdisabled != 0);
>>> }
>>>
>>> -static int tun_attach(struct tun_struct *tun, struct file *file)
>>> +static int tun_attach(struct tun_struct *tun, struct file *file, bool
>>> setiff)
>>> {
>>> struct tun_file *tfile = file->private_data;
>>> int err;
>>> @@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct
>>> file *file)
>>> if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
>>> goto out;
>>>
>>> + if (setiff && tfile->detached)
>>> + goto out;
>>> +
>>> err = -EBUSY;
>>> if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
>>> goto out;
>>> @@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
>>> *file, struct ifreq *ifr)
>>> if (err < 0)
>>> return err;
>>>
>>> - err = tun_attach(tun, file);
>>> + err = tun_attach(tun, file, true);
>>> if (err < 0)
>>> return err;
>>>
>>> @@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
>>> *file, struct ifreq *ifr)
>>> dev->features = dev->hw_features;
>>>
>>> INIT_LIST_HEAD(>disabled);
>>> - err = tun_attach(tun, file);
>>> + err = tun_attach(tun, file, true);
>>> if (err < 0)
>>> goto err_free_dev;
>>>
>>> @@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
>>> ifreq *ifr)
>>> else if (tun_not_capable(tun))
>>> ret = -EPERM;
>>> else
>>> - ret = tun_attach(tun, file);
>>> + ret = tun_attach(tun, file, false);
>>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>> tun = rcu_dereference_protected(tfile->tun,
>>> lockdep_rtnl_is_held());
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> 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 netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 12:23:19PM +0200, Michael S. Tsirkin wrote: > On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote: > > Multiqueue tun devices support detaching a tun_file from its tun_struct > > and re-attaching at a later point in time. This allows users to disable > > a specific queue temporarily. > > > > ioctl(TUNSETIFF) allows the user to specify the network interface to > > attach by name. This means the user can attempt to attach to interface > > "B" after detaching from interface "A". > > > > The driver is not designed to support this so check we are re-attaching > > to the right tun_struct. Failure to do so may lead to oops. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > This fix is for 3.8-rc. > > > > drivers/net/tun.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > > index fbd106e..cf6da6e 100644 > > --- a/drivers/net/tun.c > > +++ b/drivers/net/tun.c > > @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct > > file *file) > > err = -EINVAL; > > if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held())) > > goto out; > > + if (tfile->detached && tun != tfile->detached) > > + goto out; > > > > err = -EBUSY; > > if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1) > > -- > > 1.8.0.2 > > > I agree this is a bug but even with this patch, we still allow: > > SETIFF > SETQUEUE (DISABLED) > SETIFF > > Originally the rule always was that repeated setiff calls fail with > EINVAL. We broke that when we set tun to NULL. It's probably worth > preserving that, even if queue is disabled. Applying something like the below > instead will address this concern, won't it? Sounds good. Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 06:43:20PM +0800, Jason Wang wrote:
> On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
> > On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
> >> Multiqueue tun devices support detaching a tun_file from its tun_struct
> >> and re-attaching at a later point in time. This allows users to disable
> >> a specific queue temporarily.
> >>
> >> ioctl(TUNSETIFF) allows the user to specify the network interface to
> >> attach by name. This means the user can attempt to attach to interface
> >> "B" after detaching from interface "A".
> >>
> >> The driver is not designed to support this so check we are re-attaching
> >> to the right tun_struct. Failure to do so may lead to oops.
> >>
> >> Signed-off-by: Stefan Hajnoczi
> >> ---
> >> This fix is for 3.8-rc.
> >>
> >> drivers/net/tun.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >> index fbd106e..cf6da6e 100644
> >> --- a/drivers/net/tun.c
> >> +++ b/drivers/net/tun.c
> >> @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
> >> file *file)
> >>err = -EINVAL;
> >>if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
> >>goto out;
> >> + if (tfile->detached && tun != tfile->detached)
> >> + goto out;
> >>
> >>err = -EBUSY;
> >>if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
> >> --
> >> 1.8.0.2
> >
> > I agree this is a bug but even with this patch, we still allow:
> >
> > SETIFF
> > SETQUEUE (DISABLED)
> > SETIFF
> >
> > Originally the rule always was that repeated setiff calls fail with
> > EINVAL. We broke that when we set tun to NULL. It's probably worth
> > preserving that, even if queue is disabled. Applying something like the
> > below
> > instead will address this concern, won't it?
> >
> > Note: works with regular userspace but I didn't test
> > multiqueue userspace. What do you think.
>
> Or just fail when tun->detached is not NULL at the beginning of
> tun_set_iff()?
Yes that'll also work I think. If you prefer this pls send patch.
> > Signed-off-by: Michael S. Tsirkin
> >
> > ---
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index fbd106e..5ec8b08 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
> > BUG_ON(tun->numdisabled != 0);
> > }
> >
> > -static int tun_attach(struct tun_struct *tun, struct file *file)
> > +static int tun_attach(struct tun_struct *tun, struct file *file, bool
> > setiff)
> > {
> > struct tun_file *tfile = file->private_data;
> > int err;
> > @@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct
> > file *file)
> > if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
> > goto out;
> >
> > + if (setiff && tfile->detached)
> > + goto out;
> > +
> > err = -EBUSY;
> > if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
> > goto out;
> > @@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
> > *file, struct ifreq *ifr)
> > if (err < 0)
> > return err;
> >
> > - err = tun_attach(tun, file);
> > + err = tun_attach(tun, file, true);
> > if (err < 0)
> > return err;
> >
> > @@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
> > *file, struct ifreq *ifr)
> > dev->features = dev->hw_features;
> >
> > INIT_LIST_HEAD(>disabled);
> > - err = tun_attach(tun, file);
> > + err = tun_attach(tun, file, true);
> > if (err < 0)
> > goto err_free_dev;
> >
> > @@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
> > ifreq *ifr)
> > else if (tun_not_capable(tun))
> > ret = -EPERM;
> > else
> > - ret = tun_attach(tun, file);
> > + ret = tun_attach(tun, file, false);
> > } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> > tun = rcu_dereference_protected(tfile->tun,
> > lockdep_rtnl_is_held());
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
>> Multiqueue tun devices support detaching a tun_file from its tun_struct
>> and re-attaching at a later point in time. This allows users to disable
>> a specific queue temporarily.
>>
>> ioctl(TUNSETIFF) allows the user to specify the network interface to
>> attach by name. This means the user can attempt to attach to interface
>> "B" after detaching from interface "A".
>>
>> The driver is not designed to support this so check we are re-attaching
>> to the right tun_struct. Failure to do so may lead to oops.
>>
>> Signed-off-by: Stefan Hajnoczi
>> ---
>> This fix is for 3.8-rc.
>>
>> drivers/net/tun.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index fbd106e..cf6da6e 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
>> file *file)
>> err = -EINVAL;
>> if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
>> goto out;
>> +if (tfile->detached && tun != tfile->detached)
>> +goto out;
>>
>> err = -EBUSY;
>> if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
>> --
>> 1.8.0.2
>
> I agree this is a bug but even with this patch, we still allow:
>
> SETIFF
> SETQUEUE (DISABLED)
> SETIFF
>
> Originally the rule always was that repeated setiff calls fail with
> EINVAL. We broke that when we set tun to NULL. It's probably worth
> preserving that, even if queue is disabled. Applying something like the below
> instead will address this concern, won't it?
>
> Note: works with regular userspace but I didn't test
> multiqueue userspace. What do you think.
Or just fail when tun->detached is not NULL at the beginning of
tun_set_iff()?
> Signed-off-by: Michael S. Tsirkin
>
> ---
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fbd106e..5ec8b08 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
> BUG_ON(tun->numdisabled != 0);
> }
>
> -static int tun_attach(struct tun_struct *tun, struct file *file)
> +static int tun_attach(struct tun_struct *tun, struct file *file, bool setiff)
> {
> struct tun_file *tfile = file->private_data;
> int err;
> @@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct file
> *file)
> if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
> goto out;
>
> + if (setiff && tfile->detached)
> + goto out;
> +
> err = -EBUSY;
> if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
> goto out;
> @@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
> *file, struct ifreq *ifr)
> if (err < 0)
> return err;
>
> - err = tun_attach(tun, file);
> + err = tun_attach(tun, file, true);
> if (err < 0)
> return err;
>
> @@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
> *file, struct ifreq *ifr)
> dev->features = dev->hw_features;
>
> INIT_LIST_HEAD(>disabled);
> - err = tun_attach(tun, file);
> + err = tun_attach(tun, file, true);
> if (err < 0)
> goto err_free_dev;
>
> @@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
> ifreq *ifr)
> else if (tun_not_capable(tun))
> ret = -EPERM;
> else
> - ret = tun_attach(tun, file);
> + ret = tun_attach(tun, file, false);
> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> tun = rcu_dereference_protected(tfile->tun,
> lockdep_rtnl_is_held());
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
> Multiqueue tun devices support detaching a tun_file from its tun_struct
> and re-attaching at a later point in time. This allows users to disable
> a specific queue temporarily.
>
> ioctl(TUNSETIFF) allows the user to specify the network interface to
> attach by name. This means the user can attempt to attach to interface
> "B" after detaching from interface "A".
>
> The driver is not designed to support this so check we are re-attaching
> to the right tun_struct. Failure to do so may lead to oops.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> This fix is for 3.8-rc.
>
> drivers/net/tun.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fbd106e..cf6da6e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file
> *file)
> err = -EINVAL;
> if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
> goto out;
> + if (tfile->detached && tun != tfile->detached)
> + goto out;
>
> err = -EBUSY;
> if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
> --
> 1.8.0.2
I agree this is a bug but even with this patch, we still allow:
SETIFF
SETQUEUE (DISABLED)
SETIFF
Originally the rule always was that repeated setiff calls fail with
EINVAL. We broke that when we set tun to NULL. It's probably worth
preserving that, even if queue is disabled. Applying something like the below
instead will address this concern, won't it?
Note: works with regular userspace but I didn't test
multiqueue userspace. What do you think.
Signed-off-by: Michael S. Tsirkin
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..5ec8b08 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
BUG_ON(tun->numdisabled != 0);
}
-static int tun_attach(struct tun_struct *tun, struct file *file)
+static int tun_attach(struct tun_struct *tun, struct file *file, bool setiff)
{
struct tun_file *tfile = file->private_data;
int err;
@@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct file
*file)
if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
goto out;
+ if (setiff && tfile->detached)
+ goto out;
+
err = -EBUSY;
if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
goto out;
@@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
if (err < 0)
return err;
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err < 0)
return err;
@@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
dev->features = dev->hw_features;
INIT_LIST_HEAD(>disabled);
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err < 0)
goto err_free_dev;
@@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct ifreq
*ifr)
else if (tun_not_capable(tun))
ret = -EPERM;
else
- ret = tun_attach(tun, file);
+ ret = tun_attach(tun, file, false);
} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
tun = rcu_dereference_protected(tfile->tun,
lockdep_rtnl_is_held());
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thursday, January 10, 2013 08:59:48 AM Stefan Hajnoczi wrote: > Multiqueue tun devices support detaching a tun_file from its tun_struct > and re-attaching at a later point in time. This allows users to disable > a specific queue temporarily. > > ioctl(TUNSETIFF) allows the user to specify the network interface to > attach by name. This means the user can attempt to attach to interface > "B" after detaching from interface "A". > > The driver is not designed to support this so check we are re-attaching > to the right tun_struct. Failure to do so may lead to oops. > > Signed-off-by: Stefan Hajnoczi > --- > This fix is for 3.8-rc. > > drivers/net/tun.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index fbd106e..cf6da6e 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct > file *file) err = -EINVAL; > if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held())) > goto out; > + if (tfile->detached && tun != tfile->detached) > + goto out; > > err = -EBUSY; > if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1) Thanks. Acked-by: Jason Wang -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface "B" after detaching from interface "A". The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi --- This fix is for 3.8-rc. drivers/net/tun.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index fbd106e..cf6da6e 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file) err = -EINVAL; if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held())) goto out; + if (tfile->detached && tun != tfile->detached) + goto out; err = -EBUSY; if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1) -- 1.8.0.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
From: Stefan Hajnoczi stefa...@redhat.com Date: Thu, 10 Jan 2013 08:59:48 +0100 Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Applied. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/11/2013 06:39 AM, David Miller wrote: From: Stefan Hajnoczi stefa...@redhat.com Date: Thu, 10 Jan 2013 08:59:48 +0100 Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Applied. Hi David: Any chance that I can have a respin on this patch, there's still a bug after this patch. Or I just can send a patch on top? Thanks -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
From: Jason Wang jasow...@redhat.com Date: Fri, 11 Jan 2013 09:29:20 +0800 On 01/11/2013 06:39 AM, David Miller wrote: From: Stefan Hajnoczi stefa...@redhat.com Date: Thu, 10 Jan 2013 08:59:48 +0100 Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Applied. Hi David: Any chance that I can have a respin on this patch, there's still a bug after this patch. Or I just can send a patch on top? If I've applied it, there is no reverting. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- This fix is for 3.8-rc. drivers/net/tun.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index fbd106e..cf6da6e 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file) err = -EINVAL; if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held())) goto out; + if (tfile-detached tun != tfile-detached) + goto out; err = -EBUSY; if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1) -- 1.8.0.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thursday, January 10, 2013 08:59:48 AM Stefan Hajnoczi wrote: Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- This fix is for 3.8-rc. drivers/net/tun.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index fbd106e..cf6da6e 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file) err = -EINVAL; if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held())) goto out; + if (tfile-detached tun != tfile-detached) + goto out; err = -EBUSY; if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1) Thanks. Acked-by: Jason Wang jasow...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
Multiqueue tun devices support detaching a tun_file from its tun_struct
and re-attaching at a later point in time. This allows users to disable
a specific queue temporarily.
ioctl(TUNSETIFF) allows the user to specify the network interface to
attach by name. This means the user can attempt to attach to interface
B after detaching from interface A.
The driver is not designed to support this so check we are re-attaching
to the right tun_struct. Failure to do so may lead to oops.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
This fix is for 3.8-rc.
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..cf6da6e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file
*file)
err = -EINVAL;
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (tfile-detached tun != tfile-detached)
+ goto out;
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
--
1.8.0.2
I agree this is a bug but even with this patch, we still allow:
SETIFF
SETQUEUE (DISABLED)
SETIFF
Originally the rule always was that repeated setiff calls fail with
EINVAL. We broke that when we set tun to NULL. It's probably worth
preserving that, even if queue is disabled. Applying something like the below
instead will address this concern, won't it?
Note: works with regular userspace but I didn't test
multiqueue userspace. What do you think.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..5ec8b08 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
BUG_ON(tun-numdisabled != 0);
}
-static int tun_attach(struct tun_struct *tun, struct file *file)
+static int tun_attach(struct tun_struct *tun, struct file *file, bool setiff)
{
struct tun_file *tfile = file-private_data;
int err;
@@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct file
*file)
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (setiff tfile-detached)
+ goto out;
+
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
goto out;
@@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
if (err 0)
return err;
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
return err;
@@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
dev-features = dev-hw_features;
INIT_LIST_HEAD(tun-disabled);
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
goto err_free_dev;
@@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct ifreq
*ifr)
else if (tun_not_capable(tun))
ret = -EPERM;
else
- ret = tun_attach(tun, file);
+ ret = tun_attach(tun, file, false);
} else if (ifr-ifr_flags IFF_DETACH_QUEUE) {
tun = rcu_dereference_protected(tfile-tun,
lockdep_rtnl_is_held());
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
Multiqueue tun devices support detaching a tun_file from its tun_struct
and re-attaching at a later point in time. This allows users to disable
a specific queue temporarily.
ioctl(TUNSETIFF) allows the user to specify the network interface to
attach by name. This means the user can attempt to attach to interface
B after detaching from interface A.
The driver is not designed to support this so check we are re-attaching
to the right tun_struct. Failure to do so may lead to oops.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
This fix is for 3.8-rc.
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..cf6da6e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
err = -EINVAL;
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+if (tfile-detached tun != tfile-detached)
+goto out;
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
--
1.8.0.2
I agree this is a bug but even with this patch, we still allow:
SETIFF
SETQUEUE (DISABLED)
SETIFF
Originally the rule always was that repeated setiff calls fail with
EINVAL. We broke that when we set tun to NULL. It's probably worth
preserving that, even if queue is disabled. Applying something like the below
instead will address this concern, won't it?
Note: works with regular userspace but I didn't test
multiqueue userspace. What do you think.
Or just fail when tun-detached is not NULL at the beginning of
tun_set_iff()?
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..5ec8b08 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
BUG_ON(tun-numdisabled != 0);
}
-static int tun_attach(struct tun_struct *tun, struct file *file)
+static int tun_attach(struct tun_struct *tun, struct file *file, bool setiff)
{
struct tun_file *tfile = file-private_data;
int err;
@@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct file
*file)
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (setiff tfile-detached)
+ goto out;
+
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
goto out;
@@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
if (err 0)
return err;
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
return err;
@@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
dev-features = dev-hw_features;
INIT_LIST_HEAD(tun-disabled);
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
goto err_free_dev;
@@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
ifreq *ifr)
else if (tun_not_capable(tun))
ret = -EPERM;
else
- ret = tun_attach(tun, file);
+ ret = tun_attach(tun, file, false);
} else if (ifr-ifr_flags IFF_DETACH_QUEUE) {
tun = rcu_dereference_protected(tfile-tun,
lockdep_rtnl_is_held());
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 06:43:20PM +0800, Jason Wang wrote:
On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
Multiqueue tun devices support detaching a tun_file from its tun_struct
and re-attaching at a later point in time. This allows users to disable
a specific queue temporarily.
ioctl(TUNSETIFF) allows the user to specify the network interface to
attach by name. This means the user can attempt to attach to interface
B after detaching from interface A.
The driver is not designed to support this so check we are re-attaching
to the right tun_struct. Failure to do so may lead to oops.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
This fix is for 3.8-rc.
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..cf6da6e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
err = -EINVAL;
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (tfile-detached tun != tfile-detached)
+ goto out;
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
--
1.8.0.2
I agree this is a bug but even with this patch, we still allow:
SETIFF
SETQUEUE (DISABLED)
SETIFF
Originally the rule always was that repeated setiff calls fail with
EINVAL. We broke that when we set tun to NULL. It's probably worth
preserving that, even if queue is disabled. Applying something like the
below
instead will address this concern, won't it?
Note: works with regular userspace but I didn't test
multiqueue userspace. What do you think.
Or just fail when tun-detached is not NULL at the beginning of
tun_set_iff()?
Yes that'll also work I think. If you prefer this pls send patch.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..5ec8b08 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
BUG_ON(tun-numdisabled != 0);
}
-static int tun_attach(struct tun_struct *tun, struct file *file)
+static int tun_attach(struct tun_struct *tun, struct file *file, bool
setiff)
{
struct tun_file *tfile = file-private_data;
int err;
@@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (setiff tfile-detached)
+ goto out;
+
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
goto out;
@@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
if (err 0)
return err;
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
return err;
@@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
dev-features = dev-hw_features;
INIT_LIST_HEAD(tun-disabled);
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
goto err_free_dev;
@@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
ifreq *ifr)
else if (tun_not_capable(tun))
ret = -EPERM;
else
- ret = tun_attach(tun, file);
+ ret = tun_attach(tun, file, false);
} else if (ifr-ifr_flags IFF_DETACH_QUEUE) {
tun = rcu_dereference_protected(tfile-tun,
lockdep_rtnl_is_held());
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On Thu, Jan 10, 2013 at 12:23:19PM +0200, Michael S. Tsirkin wrote: On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote: Multiqueue tun devices support detaching a tun_file from its tun_struct and re-attaching at a later point in time. This allows users to disable a specific queue temporarily. ioctl(TUNSETIFF) allows the user to specify the network interface to attach by name. This means the user can attempt to attach to interface B after detaching from interface A. The driver is not designed to support this so check we are re-attaching to the right tun_struct. Failure to do so may lead to oops. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- This fix is for 3.8-rc. drivers/net/tun.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index fbd106e..cf6da6e 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file) err = -EINVAL; if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held())) goto out; + if (tfile-detached tun != tfile-detached) + goto out; err = -EBUSY; if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1) -- 1.8.0.2 I agree this is a bug but even with this patch, we still allow: SETIFF SETQUEUE (DISABLED) SETIFF Originally the rule always was that repeated setiff calls fail with EINVAL. We broke that when we set tun to NULL. It's probably worth preserving that, even if queue is disabled. Applying something like the below instead will address this concern, won't it? Sounds good. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
On 01/10/2013 07:07 PM, Michael S. Tsirkin wrote:
On Thu, Jan 10, 2013 at 06:43:20PM +0800, Jason Wang wrote:
On 01/10/2013 06:23 PM, Michael S. Tsirkin wrote:
On Thu, Jan 10, 2013 at 08:59:48AM +0100, Stefan Hajnoczi wrote:
Multiqueue tun devices support detaching a tun_file from its tun_struct
and re-attaching at a later point in time. This allows users to disable
a specific queue temporarily.
ioctl(TUNSETIFF) allows the user to specify the network interface to
attach by name. This means the user can attempt to attach to interface
B after detaching from interface A.
The driver is not designed to support this so check we are re-attaching
to the right tun_struct. Failure to do so may lead to oops.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
This fix is for 3.8-rc.
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..cf6da6e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -491,6 +491,8 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
err = -EINVAL;
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (tfile-detached tun != tfile-detached)
+ goto out;
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
--
1.8.0.2
I agree this is a bug but even with this patch, we still allow:
SETIFF
SETQUEUE (DISABLED)
SETIFF
Originally the rule always was that repeated setiff calls fail with
EINVAL. We broke that when we set tun to NULL. It's probably worth
preserving that, even if queue is disabled. Applying something like the
below
instead will address this concern, won't it?
Note: works with regular userspace but I didn't test
multiqueue userspace. What do you think.
Or just fail when tun-detached is not NULL at the beginning of
tun_set_iff()?
Yes that'll also work I think. If you prefer this pls send patch.
Will post a patch, thanks.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index fbd106e..5ec8b08 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -483,7 +483,7 @@ static void tun_detach_all(struct net_device *dev)
BUG_ON(tun-numdisabled != 0);
}
-static int tun_attach(struct tun_struct *tun, struct file *file)
+static int tun_attach(struct tun_struct *tun, struct file *file, bool
setiff)
{
struct tun_file *tfile = file-private_data;
int err;
@@ -492,6 +492,9 @@ static int tun_attach(struct tun_struct *tun, struct
file *file)
if (rcu_dereference_protected(tfile-tun, lockdep_rtnl_is_held()))
goto out;
+ if (setiff tfile-detached)
+ goto out;
+
err = -EBUSY;
if (!(tun-flags TUN_TAP_MQ) tun-numqueues == 1)
goto out;
@@ -1561,7 +1564,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
if (err 0)
return err;
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
return err;
@@ -1627,7 +1630,7 @@ static int tun_set_iff(struct net *net, struct file
*file, struct ifreq *ifr)
dev-features = dev-hw_features;
INIT_LIST_HEAD(tun-disabled);
- err = tun_attach(tun, file);
+ err = tun_attach(tun, file, true);
if (err 0)
goto err_free_dev;
@@ -1792,7 +1795,7 @@ static int tun_set_queue(struct file *file, struct
ifreq *ifr)
else if (tun_not_capable(tun))
ret = -EPERM;
else
- ret = tun_attach(tun, file);
+ ret = tun_attach(tun, file, false);
} else if (ifr-ifr_flags IFF_DETACH_QUEUE) {
tun = rcu_dereference_protected(tfile-tun,
lockdep_rtnl_is_held());
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
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 netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
