diff mbox series

[v3,02/26] compat_ioctl: move simple ppp command handling into driver

Message ID 20190416202013.4034148-3-arnd@arndb.de
State New
Headers show
Series compat_ioctl: cleanups | expand

Commit Message

Arnd Bergmann April 16, 2019, 8:19 p.m. UTC 
There are multiple implementations of the PPP ioctl interface in the
kernel:

- drivers/net/ppp/ppp_generic.c implements a generic interface
  for the /dev/ppp chardev used by some subdrivers.
- drivers/net/ppp/pppox.c implements a socket based interface
  for pppoe, pptp and l2tp.
- drivers/isdn/i4l/isdn_ppp.c is for the i4l ISDN stack

All ioctl commands in the respective functions are compatible between
32-bit and 64-bit kernels, so we can simply mark the handlers themselves
as compatible and stop listing the commands individually.

Four commands (PPPIOCSCOMPRESS, PPPIOCSPASS, PPPIOCSACTIVE, and
PPPIOCGIDLE) are incompatible on the user level but have a translation
handler to make them compatible. I'm simplifying that compat handling
in separate patches.

The PPPIOCGUNIT and PPPIOCGCHAN ioctl commands are special, they are
implemented on various other file descriptors, so we have to keep them
listed as COMPATIBLE_IOCTL().

For the isdn_ppp code, additional ioctl commands are needed that have
never had working compat handling, so I'm leaving that part out: If
they are remaining users of i4l's ippp, they are not using compat
mode today, and are highly unlikely in the future before the last
ISDN network gets shut down. I4L has been deprecated since 2002.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/net/ppp/ppp_generic.c |  1 +
 drivers/net/ppp/pppoe.c       |  3 +++
 drivers/net/ppp/pptp.c        |  3 +++
 fs/compat_ioctl.c             | 31 -------------------------------
 net/l2tp/l2tp_ppp.c           |  3 +++
 5 files changed, 10 insertions(+), 31 deletions(-)

-- 
2.20.0

Comments

Al Viro April 17, 2019, 9:13 p.m. UTC | #1 
On Tue, Apr 16, 2019 at 10:19:40PM +0200, Arnd Bergmann wrote:
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c

> index c708400fff4a..04252c3492ee 100644

> --- a/drivers/net/ppp/ppp_generic.c

> +++ b/drivers/net/ppp/ppp_generic.c

> @@ -899,6 +899,7 @@ static const struct file_operations ppp_device_fops = {

>  	.write		= ppp_write,

>  	.poll		= ppp_poll,

>  	.unlocked_ioctl	= ppp_ioctl,

> +	.compat_ioctl	= ppp_ioctl,


Oh?  What happens on e.g. s390 with something like PPPIOCNEWUNIT?
Current kernel:
	* no ->compat_ioctl()
	* ->unlock_ioctl() is present
	* found by compat_ioctl_check_table()
	* pass (unsigned long)compat_ptr(arg) to do_vfs_ioctl()
	* pass that to ppp_ioctl()
	* pass that to ppp_unattached_ioctl()
	* fetch int from (int __user *)compat_ptr(arg)

With your patch:
	* call ppp_ioctl()
	* pass arg to ppp_unattached_ioctl()
	* fetch int from (int __user *)arg

AFAICS, that's broken...  Looking at that ppp_ioctl(),
pointer to arch-independent type or ignored:
	PPPIOCNEWUNIT PPPIOCATTACH PPPIOCATTCHAN PPPIOCSMRU PPPIOCSFLAGS
	PPPIOCGFLAGS PPPIOCGUNIT PPPIOCSDEBUG PPPIOCSMAXCID PPPIOCCONNECT
	PPPIOCGDEBUG PPPIOCSMAXCID PPPIOCSMRRU
	PPPIOCDETACH PPPIOCDISCONN
	PPPIOCGASYNCMAP PPPIOCSASYNCMAP PPPIOCGRASYNCMAP PPPIOCSRASYNCMAP
	PPPIOCGXASYNCMAP PPPIOCSXASYNCMAP
	PPPIOCGNPMODE PPPIOCSNPMODE
pointer to struct ppp_option_data (with further pointer-chasing in it):
	PPPIOCSCOMPRESS
pointer to struct ppp_idle:
	PPPIOCGIDLE
pointer to struct sock_filter (with hidden pointer-chasing, AFAICS):
	PPPIOCSPASS PPPIOCSACTIVE

Pretty much all of them take pointers.  What's more, reaction to
unknown is -ENOTTY, not -ENOIOCTLCM, so that patch will have
prevent the translated ones from reaching do_ioctl_trans()

What am I missing here?  Why not simply do

compat_ppp_ioctl()
{
	PPPIOCSCOMPRESS32 => deal with it
	PPPIOCGIDLE32 => deal with it
	PPPIOCSPASS32 / PPPIOCSACTIVE32 => deal with it
	default: pass compat_ptr(arg) to ppp_ioctl() and be done with that
}

with BPF-related bits (both compat and native) taken to e.g. net/core/bpf-ppp.c,
picked by both generic and isdn?  IDGI...
Arnd Bergmann April 17, 2019, 10:03 p.m. UTC | #2 
On Wed, Apr 17, 2019 at 11:13 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>

> On Tue, Apr 16, 2019 at 10:19:40PM +0200, Arnd Bergmann wrote:

> > diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c

> > index c708400fff4a..04252c3492ee 100644

> > --- a/drivers/net/ppp/ppp_generic.c

> > +++ b/drivers/net/ppp/ppp_generic.c

> > @@ -899,6 +899,7 @@ static const struct file_operations ppp_device_fops = {

> >       .write          = ppp_write,

> >       .poll           = ppp_poll,

> >       .unlocked_ioctl = ppp_ioctl,

> > +     .compat_ioctl   = ppp_ioctl,

>

> Oh?  What happens on e.g. s390 with something like PPPIOCNEWUNIT?

> Current kernel:

>         * no ->compat_ioctl()

>         * ->unlock_ioctl() is present

>         * found by compat_ioctl_check_table()

>         * pass (unsigned long)compat_ptr(arg) to do_vfs_ioctl()

>         * pass that to ppp_ioctl()

>         * pass that to ppp_unattached_ioctl()

>         * fetch int from (int __user *)compat_ptr(arg)

>

> With your patch:

>         * call ppp_ioctl()

>         * pass arg to ppp_unattached_ioctl()

>         * fetch int from (int __user *)arg

>

> AFAICS, that's broken...


Correct. I had added this patch to the series from an older set of
patches (predating
the compat_ptr_ioctl() series) , and did not check for this issue again. When
I originally created the patch, I assumed that even on s390 it would be no
problem.

> Looking at that ppp_ioctl(),

> pointer to arch-independent type or ignored:

>         PPPIOCNEWUNIT PPPIOCATTACH PPPIOCATTCHAN PPPIOCSMRU PPPIOCSFLAGS

>         PPPIOCGFLAGS PPPIOCGUNIT PPPIOCSDEBUG PPPIOCSMAXCID PPPIOCCONNECT

>         PPPIOCGDEBUG PPPIOCSMAXCID PPPIOCSMRRU

>         PPPIOCDETACH PPPIOCDISCONN

>         PPPIOCGASYNCMAP PPPIOCSASYNCMAP PPPIOCGRASYNCMAP PPPIOCSRASYNCMAP

>         PPPIOCGXASYNCMAP PPPIOCSXASYNCMAP

>         PPPIOCGNPMODE PPPIOCSNPMODE

> pointer to struct ppp_option_data (with further pointer-chasing in it):

>         PPPIOCSCOMPRESS

> pointer to struct ppp_idle:

>         PPPIOCGIDLE

> pointer to struct sock_filter (with hidden pointer-chasing, AFAICS):

>         PPPIOCSPASS PPPIOCSACTIVE

>

> Pretty much all of them take pointers.  What's more, reaction to

> unknown is -ENOTTY, not -ENOIOCTLCM, so that patch will have

> prevent the translated ones from reaching do_ioctl_trans()


Good point, this patch sequence does break bisection.

> What am I missing here?  Why not simply do

>

> compat_ppp_ioctl()

> {

>         PPPIOCSCOMPRESS32 => deal with it

>         PPPIOCGIDLE32 => deal with it

>         PPPIOCSPASS32 / PPPIOCSACTIVE32 => deal with it

>         default: pass compat_ptr(arg) to ppp_ioctl() and be done with that

> }

>

> with BPF-related bits (both compat and native) taken to e.g. net/core/bpf-ppp.c,

> picked by both generic and isdn?  IDGI...


I was trying to unify the native and compat code paths as much
as possible here. Handling the four PPPIO*32 commands in
compat_ppp_ioctl would either require duplicating large chunks
of ppp_ioctl, or keeping the extra compat_alloc_user_space()
copy from the existing implementation.

I'll try to come up with a different way to structure the patches.

      Arnd
Al Viro April 17, 2019, 11:53 p.m. UTC | #3 
On Thu, Apr 18, 2019 at 12:03:07AM +0200, Arnd Bergmann wrote:
> On Wed, Apr 17, 2019 at 11:13 PM Al Viro <viro@zeniv.linux.org.uk> wrote:

> >

> > On Tue, Apr 16, 2019 at 10:19:40PM +0200, Arnd Bergmann wrote:

> > > diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c

> > > index c708400fff4a..04252c3492ee 100644

> > > --- a/drivers/net/ppp/ppp_generic.c

> > > +++ b/drivers/net/ppp/ppp_generic.c

> > > @@ -899,6 +899,7 @@ static const struct file_operations ppp_device_fops = {

> > >       .write          = ppp_write,

> > >       .poll           = ppp_poll,

> > >       .unlocked_ioctl = ppp_ioctl,

> > > +     .compat_ioctl   = ppp_ioctl,

> >

> > Oh?  What happens on e.g. s390 with something like PPPIOCNEWUNIT?

> > Current kernel:

> >         * no ->compat_ioctl()

> >         * ->unlock_ioctl() is present

> >         * found by compat_ioctl_check_table()

> >         * pass (unsigned long)compat_ptr(arg) to do_vfs_ioctl()

> >         * pass that to ppp_ioctl()

> >         * pass that to ppp_unattached_ioctl()

> >         * fetch int from (int __user *)compat_ptr(arg)

> >

> > With your patch:

> >         * call ppp_ioctl()

> >         * pass arg to ppp_unattached_ioctl()

> >         * fetch int from (int __user *)arg

> >

> > AFAICS, that's broken...

> 

> Correct. I had added this patch to the series from an older set of

> patches (predating

> the compat_ptr_ioctl() series) , and did not check for this issue again. When

> I originally created the patch, I assumed that even on s390 it would be no

> problem.

> 

> > Looking at that ppp_ioctl(),

> > pointer to arch-independent type or ignored:

> >         PPPIOCNEWUNIT PPPIOCATTACH PPPIOCATTCHAN PPPIOCSMRU PPPIOCSFLAGS

> >         PPPIOCGFLAGS PPPIOCGUNIT PPPIOCSDEBUG PPPIOCSMAXCID PPPIOCCONNECT

> >         PPPIOCGDEBUG PPPIOCSMAXCID PPPIOCSMRRU

> >         PPPIOCDETACH PPPIOCDISCONN

> >         PPPIOCGASYNCMAP PPPIOCSASYNCMAP PPPIOCGRASYNCMAP PPPIOCSRASYNCMAP

> >         PPPIOCGXASYNCMAP PPPIOCSXASYNCMAP

> >         PPPIOCGNPMODE PPPIOCSNPMODE

> > pointer to struct ppp_option_data (with further pointer-chasing in it):

> >         PPPIOCSCOMPRESS

> > pointer to struct ppp_idle:

> >         PPPIOCGIDLE

> > pointer to struct sock_filter (with hidden pointer-chasing, AFAICS):

> >         PPPIOCSPASS PPPIOCSACTIVE

> >

> > Pretty much all of them take pointers.  What's more, reaction to

> > unknown is -ENOTTY, not -ENOIOCTLCM, so that patch will have

> > prevent the translated ones from reaching do_ioctl_trans()

> 

> Good point, this patch sequence does break bisection.

> 

> > What am I missing here?  Why not simply do

> >

> > compat_ppp_ioctl()

> > {

> >         PPPIOCSCOMPRESS32 => deal with it

> >         PPPIOCGIDLE32 => deal with it

> >         PPPIOCSPASS32 / PPPIOCSACTIVE32 => deal with it

> >         default: pass compat_ptr(arg) to ppp_ioctl() and be done with that

> > }

> >

> > with BPF-related bits (both compat and native) taken to e.g. net/core/bpf-ppp.c,

> > picked by both generic and isdn?  IDGI...

> 

> I was trying to unify the native and compat code paths as much

> as possible here. Handling the four PPPIO*32 commands in

> compat_ppp_ioctl would either require duplicating large chunks

> of ppp_ioctl, or keeping the extra compat_alloc_user_space()

> copy from the existing implementation.

> 

> I'll try to come up with a different way to structure the patches.


Huh?  Instead of
        case PPPIOCSCOMPRESS:
                err = ppp_set_compress(ppp, arg);
                break;
in native, have
	struct ppp_option_data data;
...
        case PPPIOCSCOMPRESS:
		if (copy_from_user(&data, argp, sizeof(data)))
			err = -EFAULT;
		else
			err = ppp_set_compress(ppp, &data);
                break;

while in compat do
	struct ppp_option_data32 data32;

        case PPPIOCSCOMPRESS32:
		if (copy_from_user(&data32, argp, sizeof(data32)))
			err = -EFAULT;
		else
			err = ppp_set_compress(ppp, &(struct ppp_option_data){
							.ptr = compat_ptr(data32.ptr),
							.length = data32.length,
							.transmit = data32.transmit
						});
		break;

PPPIOCGIDLE is small to start with - not a lot to copy there.
And as for the filters...  What you need is something like

struct bpf_prog *get_ppp_bpf(struct sock_fprog __user *p)
{
        struct sock_fprog uprog;
	struct sock_fprog_kern fprog;
	struct sock_filter *code = NULL;
	struct bpf_prog *res;
        int err;

        if (copy_from_user(&uprog, p, sizeof(uprog)))
                return ERR_PTR(-EFAULT);

        if (!uprog.len)
                return NULL;

        fprog.len = uprog.len * sizeof(struct sock_filter);
        code = fprog.filter = memdup_user(uprog.filter, fprog.len);
        if (IS_ERR(code))
                return ERR_CAST(code);

	err = bpf_prog_create(&res, &fprog);
	kfree(code);

	if (err)
		return ERR_PTR(err);
	return res;
}
in net/core/ppp-filter.c (or in net/core/filter.c, for that matter, under
appropriate ifdef)) with obvious compat counterpart next to it.  Hell,
turn the above into

static struct bpf_prog *__get_ppp_bpf(struct sock_fprog *kp)
{
	struct sock_fprog_kern fprog;
	struct sock_filter *code = NULL;
	struct bpf_prog *res;
        int err;

        if (!kp->len)
                return NULL;

        fprog.len = kp->len * sizeof(struct sock_filter);
        code = fprog.filter = memdup_user(kp->filter, fprog.len);
        if (IS_ERR(code))
                return ERR_CAST(code);

	err = bpf_prog_create(&res, &fprog);
	kfree(code);

	if (err)
		return ERR_PTR(err);
	return res;
}

struct bpf_prog *get_ppp_bpf(struct sock_fprog __user *p)
{
        struct sock_fprog uprog;

        if (copy_from_user(&uprog, p, sizeof(uprog)))
                return ERR_PTR(-EFAULT);
	return __get_ppp_bpf(&uprog);
}

struct bpf_prog *compat_get_ppp_bpf(struct sock_fprog32 __user *p)
{
        struct sock_fprog uprog32;

        if (copy_from_user(&uprog32, p, sizeof(uprog32)))
                return ERR_PTR(-EFAULT);
	return __get_ppp_bpf(&(struct sock_fprog){
			.len = uprog32.len,
			.filter = compat_ptr(uprog32.filter)});
}

Then in native ioctl do
        case PPPIOCSPASS:
        case PPPIOCSACTIVE:
        {
                struct bpf_prog *filter = get_bpf_ppp(argp);

		if (IS_ERR(filter)) {
			err = PTR_ERR(filter);
		} else {
			struct bpf_prog **which;
			if (cmd == PPPIOCSPASS)
				which = &ppp->pass_filter;
			else
				which = &ppp->active_filter;
			ppp_lock(ppp);
			if (*which)
				bpf_prog_destroy(*which);
			*which = filter;
			ppp_unlock(ppp);
			err = 0;
		}
                break;
	}
in native and similar in compat, with get_bpf_ppp() replaced
with call of compat_get_bpf_ppp() and ioctl numbers obviously
adjusted.  All there is to it...  Helpers obviously shared
with isdn and yes, all crap gone from fs/compat_ioctl.c...

Why would you want to duplicate large chunks of anything?
The above is not even compile-tested, but...  I can put
together a patch if you wish.  Or am I missing something
here?
Al Viro April 18, 2019, 5:57 a.m. UTC | #4 
On Thu, Apr 18, 2019 at 12:53:00AM +0100, Al Viro wrote:

> Why would you want to duplicate large chunks of anything?

> The above is not even compile-tested, but...  I can put

> together a patch if you wish.  Or am I missing something

> here?


Actually, there's another broken part - pppox.  And that one
is nastier - you've just lost everything outside of protocol
family ioctls on those sockets.  Try e.g.  SIOCGIFMTU in 32bit
process on those.

We really can't reuse native ->ioctl() there - proto family
->compat_ioctl() *must* return -ENOIOCTLCMD on everything it
doesn't handle.
Arnd Bergmann April 18, 2019, 3:14 p.m. UTC | #5 
On Thu, Apr 18, 2019 at 1:53 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Apr 18, 2019 at 12:03:07AM +0200, Arnd Bergmann wrote:

> > On Wed, Apr 17, 2019 at 11:13 PM Al Viro <viro@zeniv.linux.org.uk> wrote:

> > >

> > > On Tue, Apr 16, 2019 at 10:19:40PM +0200, Arnd Bergmann wrote:

> > > > diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c

> > > > index c708400fff4a..04252c3492ee 100644

_ptr(arg) to ppp_ioctl() and be done with that
> > > }

> > >

> > > with BPF-related bits (both compat and native) taken to e.g. net/core/bpf-ppp.c,

> > > picked by both generic and isdn?  IDGI...

> >

> > I was trying to unify the native and compat code paths as much

> > as possible here. Handling the four PPPIO*32 commands in

> > compat_ppp_ioctl would either require duplicating large chunks

> > of ppp_ioctl, or keeping the extra compat_alloc_user_space()

> > copy from the existing implementation.

> >

> > I'll try to come up with a different way to structure the patches.

>

> Huh?  Instead of

>         case PPPIOCSCOMPRESS:

>                 err = ppp_set_compress(ppp, arg);

>                 break;

> in native, have

>         struct ppp_option_data data;

> ...

>         case PPPIOCSCOMPRESS:

>                 if (copy_from_user(&data, argp, sizeof(data)))

>                         err = -EFAULT;

>                 else

>                         err = ppp_set_compress(ppp, &data);

>                 break;


Right, I ended up with something similar before I saw your message.

> in native and similar in compat, with get_bpf_ppp() replaced

> with call of compat_get_bpf_ppp() and ioctl numbers obviously

> adjusted.  All there is to it...  Helpers obviously shared

> with isdn and yes, all crap gone from fs/compat_ioctl.c...


I would still leave the ISDN side alone, aside from adding
the 64-bit time_t support.

> Why would you want to duplicate large chunks of anything?

> The above is not even compile-tested, but...  I can put

> together a patch if you wish.  Or am I missing something

> here?


I expected that the ppp_compat_ioctl() function would end up
fairly complex, to duplicate the logic before the switch()/case.

What I have now is

static long ppp_compat_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
        struct ppp_file *pf;
        struct ppp *ppp;
        int err = -ENOIOCTLCMD;
        struct ppp_option_data32 data32;
        struct ppp_option_data data;
        void __user *argp = compat_ptr(arg);

        mutex_lock(&ppp_mutex);

        pf = file->private_data;
        if (!pf || pf->kind != INTERFACE)
                goto out;

        ppp = PF_TO_PPP(pf);
        switch (cmd) {
        case PPPIOCSCOMPRESS32:
                if (copy_from_user(&data32, argp, sizeof(data32))) {
                        err = -EFAULT;
                        goto out;
                }

                data.ptr = compat_ptr(data32.ptr);
                data.length = data32.length;
                data.transmit = data32.transmit;

                err = ppp_set_compress(ppp, &data);
                break;

#ifdef CONFIG_PPP_FILTER
        case PPPIOCSPASS32:
                err = compat_get_sock_fprog(&uprog, argp);
                if (err)
                        break;
                err = ppp_set_filter(ppp, &uprog, &ppp->pass_filter);
                break;

        case PPPIOCSACTIVE32:
                err = compat_get_sock_fprog(&uprog, argp);
                if (err)
                        break;
                err = ppp_set_filter(ppp, &uprog, &ppp->active_filter);
                break;
#endif /* CONFIG_PPP_FILTER */

        default:
                break;
        }

out:
        mutex_unlock(&ppp_mutex);

        if (err == -ENOIOCTLCMD)
                err = ppp_ioctl(file, cmd, (unsigned long)compat_ptr(arg));

        return err;
}

Which doesn't look nearly as bad as I had feared, but still is
a larger change to the existing code than what I had before,
so there is a bigger risk that I screwed up somewhere new.

       Arnd
diff mbox series

Patch

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index c708400fff4a..04252c3492ee 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -899,6 +899,7 @@  static const struct file_operations ppp_device_fops = {
 	.write		= ppp_write,
 	.poll		= ppp_poll,
 	.unlocked_ioctl	= ppp_ioctl,
+	.compat_ioctl	= ppp_ioctl,
 	.open		= ppp_open,
 	.release	= ppp_release,
 	.llseek		= noop_llseek,
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index c5e7435db86c..5eccb49bcd2e 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -1124,6 +1124,9 @@  static const struct proto_ops pppoe_ops = {
 	.recvmsg	= pppoe_recvmsg,
 	.mmap		= sock_no_mmap,
 	.ioctl		= pppox_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= pppox_ioctl,
+#endif
 };
 
 static const struct pppox_proto pppoe_proto = {
diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index 50c60550f295..7cf56ad50d07 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -628,6 +628,9 @@  static const struct proto_ops pptp_ops = {
 	.recvmsg    = sock_no_recvmsg,
 	.mmap       = sock_no_mmap,
 	.ioctl      = pppox_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = pppox_ioctl,
+#endif
 };
 
 static const struct pppox_proto pppox_pptp_proto = {
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index f1065d116b55..54f26a9fa9f2 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -608,39 +608,8 @@  COMPATIBLE_IOCTL(SG_SET_KEEP_ORPHAN)
 COMPATIBLE_IOCTL(SG_GET_KEEP_ORPHAN)
 #endif
 /* PPP stuff */
-COMPATIBLE_IOCTL(PPPIOCGFLAGS)
-COMPATIBLE_IOCTL(PPPIOCSFLAGS)
-COMPATIBLE_IOCTL(PPPIOCGASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSASYNCMAP)
 COMPATIBLE_IOCTL(PPPIOCGUNIT)
-COMPATIBLE_IOCTL(PPPIOCGRASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSRASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCGMRU)
-COMPATIBLE_IOCTL(PPPIOCSMRU)
-COMPATIBLE_IOCTL(PPPIOCSMAXCID)
-COMPATIBLE_IOCTL(PPPIOCGXASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCSXASYNCMAP)
-COMPATIBLE_IOCTL(PPPIOCXFERUNIT)
-/* PPPIOCSCOMPRESS is translated */
-COMPATIBLE_IOCTL(PPPIOCGNPMODE)
-COMPATIBLE_IOCTL(PPPIOCSNPMODE)
-COMPATIBLE_IOCTL(PPPIOCGDEBUG)
-COMPATIBLE_IOCTL(PPPIOCSDEBUG)
-/* PPPIOCSPASS is translated */
-/* PPPIOCSACTIVE is translated */
-/* PPPIOCGIDLE is translated */
-COMPATIBLE_IOCTL(PPPIOCNEWUNIT)
-COMPATIBLE_IOCTL(PPPIOCATTACH)
-COMPATIBLE_IOCTL(PPPIOCDETACH)
-COMPATIBLE_IOCTL(PPPIOCSMRRU)
-COMPATIBLE_IOCTL(PPPIOCCONNECT)
-COMPATIBLE_IOCTL(PPPIOCDISCONN)
-COMPATIBLE_IOCTL(PPPIOCATTCHAN)
 COMPATIBLE_IOCTL(PPPIOCGCHAN)
-COMPATIBLE_IOCTL(PPPIOCGL2TPSTATS)
-/* PPPOX */
-COMPATIBLE_IOCTL(PPPOEIOCSFWD32)
-COMPATIBLE_IOCTL(PPPOEIOCDFWD)
 /* Big A */
 /* sparc only */
 /* Big Q for sound/OSS */
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 04d9946dcdba..8ef66513fbe0 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1686,6 +1686,9 @@  static const struct proto_ops pppol2tp_ops = {
 	.recvmsg	= pppol2tp_recvmsg,
 	.mmap		= sock_no_mmap,
 	.ioctl		= pppox_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = pppox_ioctl,
+#endif
 };
 
 static const struct pppox_proto pppol2tp_proto = {