* Re: TUNSETIFF: data not copied back to userspace
2005-02-14 19:41 TUNSETIFF: data not copied back to userspace Michael Tautschnig
@ 2005-02-14 20:45 ` Eric Brower
2005-02-14 22:59 ` Michael Tautschnig
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Brower @ 2005-02-14 20:45 UTC (permalink / raw)
To: sparclinux
Take a look at the dev_ifsioc() function in ioctl32.c-- it does not
copy the data back to userspace except for SIOCSIFMAP, and judging
from the strict definition of TUNSETIFF it should not be required to
do so.
I think if this is functionality that is required (I see no TUNGETIFF,
but I don't play with this stuff enough to know) the directionality of
the ioctl definition should be _IOWR rather than _IOR; in any case,
the "argument" field should not be "int" but rather "struct ifreq*"
in if_tun.h. Likely, neither will change for historical reasons....
E
On Mon, 14 Feb 2005 20:41:33 +0100 (CET), Michael Tautschnig
<michael.tautschnig@zt-consulting.com> wrote:
> Hello!
>
> Thanks for the information regarding userland/kernelspace - I think I
> understood ...
>
> The reason of my question were problems using the tun/tap interface - I've
> got an application (namely l2tpns) demanding the ifreq-structure being
> copied back to userspace - which AFAIK doesn't happen in case of the
> TUNSETIFF-ioctl. Is this a bug or are there any reasons why this shouldn't
> happen in dev_ifsioc ?
>
> Thanks,
> Michael
>
> -
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
E
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: TUNSETIFF: data not copied back to userspace
2005-02-14 19:41 TUNSETIFF: data not copied back to userspace Michael Tautschnig
2005-02-14 20:45 ` Eric Brower
@ 2005-02-14 22:59 ` Michael Tautschnig
2005-02-14 23:02 ` David S. Miller
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Michael Tautschnig @ 2005-02-14 22:59 UTC (permalink / raw)
To: sparclinux
> Take a look at the dev_ifsioc() function in ioctl32.c-- it does not
> copy the data back to userspace except for SIOCSIFMAP, and judging
> from the strict definition of TUNSETIFF it should not be required to
> do so.
IMHO this isn't true -
switch (cmd) {
case SIOCGIFFLAGS:
case SIOCGIFMETRIC:
case SIOCGIFMTU:
case SIOCGIFMEM:
case SIOCGIFHWADDR:
case SIOCGIFINDEX:
case SIOCGIFADDR:
case SIOCGIFBRDADDR:
case SIOCGIFDSTADDR:
case SIOCGIFNETMASK:
case SIOCGIFTXQLEN:
if (copy_to_user(uifr32, &ifr, sizeof(*uifr32)))
return -EFAULT;
break;
case SIOCGIFMAP:
- the (modified) struct is copied in all cases listed above. But what do
you mean by "the strict definition of TUNSETIFF"?
>
> I think if this is functionality that is required (I see no TUNGETIFF,
> but I don't play with this stuff enough to know) the directionality of
> the ioctl definition should be _IOWR rather than _IOR; in any case,
> the "argument" field should not be "int" but rather "struct ifreq*"
> in if_tun.h. Likely, neither will change for historical reasons....
>
The idea behind all that is, that the application needs (?) to know the
name of the device (tunX) to do further ioctls on that device! Of course,
it could be hardcoded, but that doesn't seem as nice ...
Why should be be _IOWR there?
Thanks,
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: TUNSETIFF: data not copied back to userspace
2005-02-14 19:41 TUNSETIFF: data not copied back to userspace Michael Tautschnig
2005-02-14 20:45 ` Eric Brower
2005-02-14 22:59 ` Michael Tautschnig
@ 2005-02-14 23:02 ` David S. Miller
2005-02-14 23:18 ` Michael Tautschnig
2005-02-14 23:30 ` David S. Miller
4 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2005-02-14 23:02 UTC (permalink / raw)
To: sparclinux
On Mon, 14 Feb 2005 23:59:32 +0100 (CET)
Michael Tautschnig <michael.tautschnig@zt-consulting.com> wrote:
> switch (cmd) {
> case SIOCGIFFLAGS:
> case SIOCGIFMETRIC:
> case SIOCGIFMTU:
> case SIOCGIFMEM:
> case SIOCGIFHWADDR:
> case SIOCGIFINDEX:
> case SIOCGIFADDR:
> case SIOCGIFBRDADDR:
> case SIOCGIFDSTADDR:
> case SIOCGIFNETMASK:
> case SIOCGIFTXQLEN:
> if (copy_to_user(uifr32, &ifr, sizeof(*uifr32)))
> return -EFAULT;
> break;
> case SIOCGIFMAP:
>
> - the (modified) struct is copied in all cases listed above. But what do
> you mean by "the strict definition of TUNSETIFF"?
So add "case TUNSETIFF" to the switch statement above.
You discovered the fix for your own bug, so why not implement
it and submit the resulting patch here? :-)
> Why should be be _IOWR there?
ioctl's are mostly defined in with a set of macros that describe
whether data goes in or out (or both) from/to userspace. The
_IORW macros set the direction to indicate that data goes both
in and out.
These macros are defined in include/asm/ioctl.h
include/linux/if_tun.h defines TUNSETIFF using the _IOW() macro.
Which indicates that data only comes in from userspace. This definition
is incorrect, it should use _IORW() as Eric stated, but it can't be changed
because this ioctl value is hardcoded into many existing applications which
would break if we "fixed" this.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: TUNSETIFF: data not copied back to userspace
2005-02-14 19:41 TUNSETIFF: data not copied back to userspace Michael Tautschnig
` (2 preceding siblings ...)
2005-02-14 23:02 ` David S. Miller
@ 2005-02-14 23:18 ` Michael Tautschnig
2005-02-14 23:30 ` David S. Miller
4 siblings, 0 replies; 6+ messages in thread
From: Michael Tautschnig @ 2005-02-14 23:18 UTC (permalink / raw)
To: sparclinux
[...]
>
> So add "case TUNSETIFF" to the switch statement above.
> You discovered the fix for your own bug, so why not implement
> it and submit the resulting patch here? :-)
It might have been intentionally left out - but here we go:
--- linux-2.6.10/fs/compat_ioctl.c 2005-02-15 00:09:53.969591354 +0100
+++ linux-2.6.10.patched/fs/compat_ioctl.c 2005-02-15 00:08:26.078059666 +0100
@@ -696,6 +696,7 @@
case SIOCGIFDSTADDR:
case SIOCGIFNETMASK:
case SIOCGIFTXQLEN:
+ case TUNSETIFF:
if (copy_to_user(uifr32, &ifr, sizeof(*uifr32)))
return -EFAULT;
break;
:-))
Actually, it did the trick - l2tpns works fine now.
[...]
>
> include/linux/if_tun.h defines TUNSETIFF using the _IOW() macro.
> Which indicates that data only comes in from userspace. This definition
> is incorrect, it should use _IORW() as Eric stated, but it can't be changed
> because this ioctl value is hardcoded into many existing applications which
> would break if we "fixed" this.
>
So the preferred behaviour would be: Add an interface-specification to
l2tpns' config - or will the above "patch" ever be included in the
official kernel?
Thanks for your help,
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: TUNSETIFF: data not copied back to userspace
2005-02-14 19:41 TUNSETIFF: data not copied back to userspace Michael Tautschnig
` (3 preceding siblings ...)
2005-02-14 23:18 ` Michael Tautschnig
@ 2005-02-14 23:30 ` David S. Miller
4 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2005-02-14 23:30 UTC (permalink / raw)
To: sparclinux
On Tue, 15 Feb 2005 00:18:08 +0100 (CET)
Michael Tautschnig <michael.tautschnig@zt-consulting.com> wrote:
> So the preferred behaviour would be: Add an interface-specification to
> l2tpns' config - or will the above "patch" ever be included in the
> official kernel?
I put the patch into my tree just now, I'll push it upstream.
^ permalink raw reply [flat|nested] 6+ messages in thread