Support for handling the PPPOEIOCSFWD ioctl in compat mode was added in
linux-2.5.69 along with hundreds of other commands, but was always broken
sincen only the structure is compatible, but the command number is not,
due to the size being sizeof(size_t), or at first sizeof(sizeof((struct
sockaddr_pppox)), which is different on 64-bit architectures.
Fix it by defining a separate command code that matches the 32-bit
version, and marking that one as compatible.
This should apply to all stable kernels.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ppp/pppoe.c | 4 ++++
fs/compat_ioctl.c | 2 +-
include/linux/if_pppox.h | 2 ++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index ce61231e96ea..d1c3f9292c54 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -57,6 +57,7 @@
*
*/
+#include <linux/compat.h>
#include <linux/string.h>
#include <linux/module.h>
#include <linux/kernel.h>
@@ -780,6 +781,9 @@ static int pppoe_ioctl(struct socket *sock, unsigned int cmd,
err = 0;
break;
+#ifdef CONFIG_COMPAT
+ case PPPOEIOCSFWD32:
+#endif
case PPPOEIOCSFWD:
{
struct pppox_sock *relay_po;
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index a9b00942e87d..a8bb193fdfd5 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -895,7 +895,7 @@ COMPATIBLE_IOCTL(PPPIOCATTCHAN)
COMPATIBLE_IOCTL(PPPIOCGCHAN)
COMPATIBLE_IOCTL(PPPIOCGL2TPSTATS)
/* PPPOX */
-COMPATIBLE_IOCTL(PPPOEIOCSFWD)
+COMPATIBLE_IOCTL(PPPOEIOCSFWD32)
COMPATIBLE_IOCTL(PPPOEIOCDFWD)
/* Big A */
/* sparc only */
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index ba7a9b0c7c57..d221f1465f41 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -85,6 +85,8 @@ extern void unregister_pppox_proto(int proto_num);
extern void pppox_unbind_sock(struct sock *sk);/* delete ppp-channel binding */
extern int pppox_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+#define PPPOEIOCSFWD32 _IOW(0xB1 ,0, compat_size_t)
+
/* PPPoX socket states */
enum {
PPPOX_NONE = 0, /* initial state */
--
2.18.0
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 <[email protected]>
---
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(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 02ad03a2fab7..41a6e9851a4a 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 d1c3f9292c54..25174fa7a470 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -1120,6 +1120,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 67ffe74747a1..3b5ab3f6745c 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -632,6 +632,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 a8bb193fdfd5..142ca673b9cc 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -864,39 +864,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 = {
--
2.18.0
PPPIOCSCOMPRESS is only implemented in ppp_generic, so it's best to move
the compat handling there. My first approach was to keep it in a new
ppp_compat_ioctl() function, but it turned out to be much simpler to do
it in the regular ioctl handler, by allowing both structure layouts to
be handled directly there.
Aside from moving the code to the right place, this also avoids
a round-trip through compat_alloc_user_space() allocated memory.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ppp/ppp_generic.c | 40 ++++++++++++++++++++++++++++++-----
fs/compat_ioctl.c | 32 ----------------------------
2 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 41a6e9851a4a..8dfe8d47df95 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -274,7 +274,7 @@ static void ppp_mp_insert(struct ppp *ppp, struct sk_buff *skb);
static struct sk_buff *ppp_mp_reconstruct(struct ppp *ppp);
static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb);
#endif /* CONFIG_PPP_MULTILINK */
-static int ppp_set_compress(struct ppp *ppp, unsigned long arg);
+static int ppp_set_compress(struct ppp *ppp, unsigned long arg, bool compat);
static void ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound);
static void ppp_ccp_closed(struct ppp *ppp);
static struct compressor *find_compressor(int type);
@@ -557,6 +557,15 @@ static __poll_t ppp_poll(struct file *file, poll_table *wait)
return mask;
}
+#ifdef CONFIG_COMPAT
+struct ppp_option_data32 {
+ compat_caddr_t ptr;
+ u32 length;
+ compat_int_t transmit;
+};
+#define PPPIOCSCOMPRESS32 _IOW('t', 77, struct ppp_option_data32)
+#endif
+
#ifdef CONFIG_PPP_FILTER
static int get_filter(void __user *arg, struct sock_filter **p)
{
@@ -683,8 +692,14 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case PPPIOCSCOMPRESS:
- err = ppp_set_compress(ppp, arg);
+ err = ppp_set_compress(ppp, arg, false);
+ break;
+
+#ifdef CONFIG_COMPAT
+ case PPPIOCSCOMPRESS32:
+ err = ppp_set_compress(ppp, arg, true);
break;
+#endif
case PPPIOCGUNIT:
if (put_user(ppp->file.index, p))
@@ -2691,7 +2706,7 @@ ppp_output_wakeup(struct ppp_channel *chan)
/* Process the PPPIOCSCOMPRESS ioctl. */
static int
-ppp_set_compress(struct ppp *ppp, unsigned long arg)
+ppp_set_compress(struct ppp *ppp, unsigned long arg, bool compat)
{
int err;
struct compressor *cp, *ocomp;
@@ -2700,8 +2715,23 @@ ppp_set_compress(struct ppp *ppp, unsigned long arg)
unsigned char ccp_option[CCP_MAX_OPTION_LENGTH];
err = -EFAULT;
- if (copy_from_user(&data, (void __user *) arg, sizeof(data)))
- goto out;
+#ifdef CONFIG_COMPAT
+ if (compat) {
+ struct ppp_option_data32 data32;
+
+ if (copy_from_user(&data32, (void __user *) arg,
+ sizeof(data32)))
+ goto out;
+
+ data.ptr = compat_ptr(data32.ptr);
+ data.length = data32.length;
+ data.transmit = data32.transmit;
+ } else
+#endif
+ {
+ if (copy_from_user(&data, (void __user *) arg, sizeof(data)))
+ goto out;
+ }
if (data.length > CCP_MAX_OPTION_LENGTH)
goto out;
if (copy_from_user(ccp_option, (void __user *) data.ptr, data.length))
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 142ca673b9cc..f518dc174dc7 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -413,13 +413,6 @@ static int ppp_sock_fprog_ioctl_trans(struct file *file,
return do_ioctl(file, cmd, (unsigned long) u_fprog64);
}
-struct ppp_option_data32 {
- compat_caddr_t ptr;
- u32 length;
- compat_int_t transmit;
-};
-#define PPPIOCSCOMPRESS32 _IOW('t', 77, struct ppp_option_data32)
-
struct ppp_idle32 {
compat_time_t xmit_idle;
compat_time_t recv_idle;
@@ -447,29 +440,6 @@ static int ppp_gidle(struct file *file, unsigned int cmd,
return err;
}
-static int ppp_scompress(struct file *file, unsigned int cmd,
- struct ppp_option_data32 __user *odata32)
-{
- struct ppp_option_data __user *odata;
- __u32 data;
- void __user *datap;
-
- odata = compat_alloc_user_space(sizeof(*odata));
-
- if (get_user(data, &odata32->ptr))
- return -EFAULT;
-
- datap = compat_ptr(data);
- if (put_user(datap, &odata->ptr))
- return -EFAULT;
-
- if (copy_in_user(&odata->length, &odata32->length,
- sizeof(__u32) + sizeof(int)))
- return -EFAULT;
-
- return do_ioctl(file, PPPIOCSCOMPRESS, (unsigned long) odata);
-}
-
#ifdef CONFIG_BLOCK
struct mtget32 {
compat_long_t mt_type;
@@ -1248,8 +1218,6 @@ static long do_ioctl_trans(unsigned int cmd,
switch (cmd) {
case PPPIOCGIDLE32:
return ppp_gidle(file, cmd, argp);
- case PPPIOCSCOMPRESS32:
- return ppp_scompress(file, cmd, argp);
case PPPIOCSPASS32:
case PPPIOCSACTIVE32:
return ppp_sock_fprog_ioctl_trans(file, cmd, argp);
--
2.18.0
PPPIOCSPASS and PPPIOCSACTIVE are implemented in ppp_generic and isdn_ppp,
but the latter one doesn't work for compat mode in general, so we can
move these two into the generic code.
Again, the best implementation I could come up with was to merge
the compat handling into the regular ppp_ioctl() function and
treating all ioctl commands as compatible.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ppp/ppp_generic.c | 39 ++++++++++++++++++++++++++++++-----
fs/compat_ioctl.c | 37 ---------------------------------
2 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 8dfe8d47df95..3a7aa2eed415 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -22,6 +22,7 @@
* ==FILEVERSION 20041108==
*/
+#include <linux/compat.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/sched/signal.h>
@@ -567,14 +568,36 @@ struct ppp_option_data32 {
#endif
#ifdef CONFIG_PPP_FILTER
-static int get_filter(void __user *arg, struct sock_filter **p)
+#ifdef CONFIG_COMPAT
+struct sock_fprog32 {
+ unsigned short len;
+ compat_caddr_t filter;
+};
+#define PPPIOCSPASS32 _IOW('t', 71, struct sock_fprog32)
+#define PPPIOCSACTIVE32 _IOW('t', 70, struct sock_fprog32)
+#endif
+
+static int get_filter(void __user *arg, struct sock_filter **p, bool compat)
{
struct sock_fprog uprog;
struct sock_filter *code = NULL;
int len;
- if (copy_from_user(&uprog, arg, sizeof(uprog)))
- return -EFAULT;
+#ifdef CONFIG_COMPAT
+ if (compat) {
+ struct sock_fprog32 uprog32;
+
+ if (copy_from_user(&uprog32, arg, sizeof(uprog32)))
+ return -EFAULT;
+
+ uprog.len = uprog32.len;
+ uprog.filter = compat_ptr(uprog32.filter);
+ } else
+#endif
+ {
+ if (copy_from_user(&uprog, arg, sizeof(uprog)))
+ return -EFAULT;
+ }
if (!uprog.len) {
*p = NULL;
@@ -772,10 +795,13 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
#ifdef CONFIG_PPP_FILTER
case PPPIOCSPASS:
+#ifdef CONFIG_COMPAT
+ case PPPIOCSPASS32:
+#endif
{
struct sock_filter *code;
- err = get_filter(argp, &code);
+ err = get_filter(argp, &code, cmd != PPPIOCSPASS);
if (err >= 0) {
struct bpf_prog *pass_filter = NULL;
struct sock_fprog_kern fprog = {
@@ -798,10 +824,13 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
}
case PPPIOCSACTIVE:
+#ifdef CONFIG_COMPAT
+ case PPPIOCSACTIVE32:
+#endif
{
struct sock_filter *code;
- err = get_filter(argp, &code);
+ err = get_filter(argp, &code, cmd != PPPIOCSACTIVE);
if (err >= 0) {
struct bpf_prog *active_filter = NULL;
struct sock_fprog_kern fprog = {
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index f518dc174dc7..258c6938e80a 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -379,40 +379,6 @@ static int sg_grt_trans(struct file *file,
}
#endif /* CONFIG_BLOCK */
-struct sock_fprog32 {
- unsigned short len;
- compat_caddr_t filter;
-};
-
-#define PPPIOCSPASS32 _IOW('t', 71, struct sock_fprog32)
-#define PPPIOCSACTIVE32 _IOW('t', 70, struct sock_fprog32)
-
-static int ppp_sock_fprog_ioctl_trans(struct file *file,
- unsigned int cmd, struct sock_fprog32 __user *u_fprog32)
-{
- struct sock_fprog __user *u_fprog64 = compat_alloc_user_space(sizeof(struct sock_fprog));
- void __user *fptr64;
- u32 fptr32;
- u16 flen;
-
- if (get_user(flen, &u_fprog32->len) ||
- get_user(fptr32, &u_fprog32->filter))
- return -EFAULT;
-
- fptr64 = compat_ptr(fptr32);
-
- if (put_user(flen, &u_fprog64->len) ||
- put_user(fptr64, &u_fprog64->filter))
- return -EFAULT;
-
- if (cmd == PPPIOCSPASS32)
- cmd = PPPIOCSPASS;
- else
- cmd = PPPIOCSACTIVE;
-
- return do_ioctl(file, cmd, (unsigned long) u_fprog64);
-}
-
struct ppp_idle32 {
compat_time_t xmit_idle;
compat_time_t recv_idle;
@@ -1218,9 +1184,6 @@ static long do_ioctl_trans(unsigned int cmd,
switch (cmd) {
case PPPIOCGIDLE32:
return ppp_gidle(file, cmd, argp);
- case PPPIOCSPASS32:
- case PPPIOCSACTIVE32:
- return ppp_sock_fprog_ioctl_trans(file, cmd, argp);
#ifdef CONFIG_BLOCK
case SG_IO:
return sg_ioctl_trans(file, cmd, argp);
--
2.18.0
The ppp_idle structure is defined in terms of __kernel_time_t, which is
defined as 'long' on all architectures, and this usage is not affected
by the y2038 problem since it transports a time interval rather than an
absolute time.
However, the ppp user space defines the same structure as time_t, which
may be 64-bit wide on new libc versions even on 32-bit architectures.
It's easy enough to just handle both possible structure layouts on
all architectures, to deal with the possibility that a user space ppp
implementation comes with its own ppp_idle structure definition, as well
as to document the fact that the driver is y2038-safe.
Doing this also avoids the need for a special compat mode translation,
since 32-bit and 64-bit kernels now support the same interfaces.
Signed-off-by: Arnd Bergmann <[email protected]>
---
Documentation/networking/ppp_generic.txt | 2 ++
drivers/isdn/i4l/isdn_ppp.c | 14 ++++++++---
drivers/net/ppp/ppp_generic.c | 18 ++++++++++----
fs/compat_ioctl.c | 31 ------------------------
include/uapi/linux/ppp-ioctl.h | 2 ++
include/uapi/linux/ppp_defs.h | 14 +++++++++++
6 files changed, 42 insertions(+), 39 deletions(-)
diff --git a/Documentation/networking/ppp_generic.txt b/Documentation/networking/ppp_generic.txt
index 61daf4b39600..fd563aff5fc9 100644
--- a/Documentation/networking/ppp_generic.txt
+++ b/Documentation/networking/ppp_generic.txt
@@ -378,6 +378,8 @@ an interface unit are:
CONFIG_PPP_FILTER option is enabled, the set of packets which reset
the transmit and receive idle timers is restricted to those which
pass the `active' packet filter.
+ Two versions of this command exist, to deal with user space
+ expecting times as either 32-bit or 64-bit time_t seconds.
* PPPIOCSMAXCID sets the maximum connection-ID parameter (and thus the
number of connection slots) for the TCP header compressor and
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index a7b275ea5de1..1f17126c5fa4 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -543,11 +543,19 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
}
is->pppcfg = val;
break;
- case PPPIOCGIDLE: /* get idle time information */
+ case PPPIOCGIDLE32: /* get idle time information */
if (lp) {
- struct ppp_idle pidle;
+ struct ppp_idle32 pidle;
pidle.xmit_idle = pidle.recv_idle = lp->huptimer;
- if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle))))
+ if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle32))))
+ return r;
+ }
+ break;
+ case PPPIOCGIDLE64: /* get idle time information */
+ if (lp) {
+ struct ppp_idle64 pidle;
+ pidle.xmit_idle = pidle.recv_idle = lp->huptimer;
+ if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle64))))
return r;
}
break;
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 3a7aa2eed415..c8b8aa071140 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -619,7 +619,8 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct ppp_file *pf;
struct ppp *ppp;
int err = -EFAULT, val, val2, i;
- struct ppp_idle idle;
+ struct ppp_idle32 idle32;
+ struct ppp_idle64 idle64;
struct npioctl npi;
int unit, cflags;
struct slcompress *vj;
@@ -743,10 +744,17 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
err = 0;
break;
- case PPPIOCGIDLE:
- idle.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
- idle.recv_idle = (jiffies - ppp->last_recv) / HZ;
- if (copy_to_user(argp, &idle, sizeof(idle)))
+ case PPPIOCGIDLE32:
+ idle32.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
+ idle32.recv_idle = (jiffies - ppp->last_recv) / HZ;
+ if (copy_to_user(argp, &idle32, sizeof(idle32)))
+ err = 0;
+ break;
+
+ case PPPIOCGIDLE64:
+ idle64.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
+ idle64.recv_idle = (jiffies - ppp->last_recv) / HZ;
+ if (copy_to_user(argp, &idle32, sizeof(idle32)))
break;
err = 0;
break;
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 258c6938e80a..208ff51f3ed9 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -377,36 +377,7 @@ static int sg_grt_trans(struct file *file,
}
return err;
}
-#endif /* CONFIG_BLOCK */
-
-struct ppp_idle32 {
- compat_time_t xmit_idle;
- compat_time_t recv_idle;
-};
-#define PPPIOCGIDLE32 _IOR('t', 63, struct ppp_idle32)
-
-static int ppp_gidle(struct file *file, unsigned int cmd,
- struct ppp_idle32 __user *idle32)
-{
- struct ppp_idle __user *idle;
- __kernel_time_t xmit, recv;
- int err;
-
- idle = compat_alloc_user_space(sizeof(*idle));
-
- err = do_ioctl(file, PPPIOCGIDLE, (unsigned long) idle);
- if (!err) {
- if (get_user(xmit, &idle->xmit_idle) ||
- get_user(recv, &idle->recv_idle) ||
- put_user(xmit, &idle32->xmit_idle) ||
- put_user(recv, &idle32->recv_idle))
- err = -EFAULT;
- }
- return err;
-}
-
-#ifdef CONFIG_BLOCK
struct mtget32 {
compat_long_t mt_type;
compat_long_t mt_resid;
@@ -1182,8 +1153,6 @@ static long do_ioctl_trans(unsigned int cmd,
void __user *argp = compat_ptr(arg);
switch (cmd) {
- case PPPIOCGIDLE32:
- return ppp_gidle(file, cmd, argp);
#ifdef CONFIG_BLOCK
case SG_IO:
return sg_ioctl_trans(file, cmd, argp);
diff --git a/include/uapi/linux/ppp-ioctl.h b/include/uapi/linux/ppp-ioctl.h
index 88b5f9990320..7bd2a5a75348 100644
--- a/include/uapi/linux/ppp-ioctl.h
+++ b/include/uapi/linux/ppp-ioctl.h
@@ -104,6 +104,8 @@ struct pppol2tp_ioc_stats {
#define PPPIOCGDEBUG _IOR('t', 65, int) /* Read debug level */
#define PPPIOCSDEBUG _IOW('t', 64, int) /* Set debug level */
#define PPPIOCGIDLE _IOR('t', 63, struct ppp_idle) /* get idle time */
+#define PPPIOCGIDLE32 _IOR('t', 63, struct ppp_idle32) /* 32-bit times */
+#define PPPIOCGIDLE64 _IOR('t', 63, struct ppp_idle64) /* 64-bit times */
#define PPPIOCNEWUNIT _IOWR('t', 62, int) /* create new ppp unit */
#define PPPIOCATTACH _IOW('t', 61, int) /* attach to ppp unit */
#define PPPIOCDETACH _IOW('t', 60, int) /* obsolete, do not use */
diff --git a/include/uapi/linux/ppp_defs.h b/include/uapi/linux/ppp_defs.h
index fff51b91b409..0039fa39a358 100644
--- a/include/uapi/linux/ppp_defs.h
+++ b/include/uapi/linux/ppp_defs.h
@@ -142,10 +142,24 @@ struct ppp_comp_stats {
/*
* The following structure records the time in seconds since
* the last NP packet was sent or received.
+ *
+ * Linux implements both 32-bit and 64-bit time_t versions
+ * for compatibility with user space that defines ppp_idle
+ * based on the libc time_t.
*/
struct ppp_idle {
__kernel_time_t xmit_idle; /* time since last NP packet sent */
__kernel_time_t recv_idle; /* time since last NP packet received */
};
+struct ppp_idle32 {
+ __s32 xmit_idle; /* time since last NP packet sent */
+ __s32 recv_idle; /* time since last NP packet received */
+};
+
+struct ppp_idle64 {
+ __s64 xmit_idle; /* time since last NP packet sent */
+ __s64 recv_idle; /* time since last NP packet received */
+};
+
#endif /* _UAPI_PPP_DEFS_H_ */
--
2.18.0
On Wed, Aug 29, 2018 at 04:03:28PM +0200, Arnd Bergmann wrote:
> PPPIOCSCOMPRESS is only implemented in ppp_generic, so it's best to move
> the compat handling there. My first approach was to keep it in a new
> ppp_compat_ioctl() function, but it turned out to be much simpler to do
> it in the regular ioctl handler, by allowing both structure layouts to
> be handled directly there.
>
Acked-by: Guillaume Nault <[email protected]>
On Wed, Aug 29, 2018 at 04:03:29PM +0200, Arnd Bergmann wrote:
> PPPIOCSPASS and PPPIOCSACTIVE are implemented in ppp_generic and isdn_ppp,
> but the latter one doesn't work for compat mode in general, so we can
> move these two into the generic code.
>
> Again, the best implementation I could come up with was to merge
> the compat handling into the regular ppp_ioctl() function and
> treating all ioctl commands as compatible.
>
Acked-by: Guillaume Nault <[email protected]>
On Wed, Aug 29, 2018 at 04:03:26PM +0200, Arnd Bergmann wrote:
> Support for handling the PPPOEIOCSFWD ioctl in compat mode was added in
> linux-2.5.69 along with hundreds of other commands, but was always broken
> sincen only the structure is compatible, but the command number is not,
> due to the size being sizeof(size_t), or at first sizeof(sizeof((struct
> sockaddr_pppox)), which is different on 64-bit architectures.
>
And the implementation was broken until 2016 (see 29e73269aa4d ("pppoe:
fix reference counting in PPPoE proxy")), and nobody ever noticed. I
should probably have removed this ioctl entirely instead of fixing it.
Clearly, it has never been used.
If you think it's worth fixing (as opposed to dropping this ioctl or
its compat mode), then,
Acked-by: Guillaume Nault <[email protected]>
On Wed, Aug 29, 2018 at 04:03:30PM +0200, Arnd Bergmann wrote:
> The ppp_idle structure is defined in terms of __kernel_time_t, which is
> defined as 'long' on all architectures, and this usage is not affected
> by the y2038 problem since it transports a time interval rather than an
> absolute time.
>
> However, the ppp user space defines the same structure as time_t, which
> may be 64-bit wide on new libc versions even on 32-bit architectures.
>
> It's easy enough to just handle both possible structure layouts on
> all architectures, to deal with the possibility that a user space ppp
> implementation comes with its own ppp_idle structure definition, as well
> as to document the fact that the driver is y2038-safe.
>
> Doing this also avoids the need for a special compat mode translation,
> since 32-bit and 64-bit kernels now support the same interfaces.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> Documentation/networking/ppp_generic.txt | 2 ++
> drivers/isdn/i4l/isdn_ppp.c | 14 ++++++++---
> drivers/net/ppp/ppp_generic.c | 18 ++++++++++----
> fs/compat_ioctl.c | 31 ------------------------
> include/uapi/linux/ppp-ioctl.h | 2 ++
> include/uapi/linux/ppp_defs.h | 14 +++++++++++
> 6 files changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/Documentation/networking/ppp_generic.txt b/Documentation/networking/ppp_generic.txt
> index 61daf4b39600..fd563aff5fc9 100644
> --- a/Documentation/networking/ppp_generic.txt
> +++ b/Documentation/networking/ppp_generic.txt
> @@ -378,6 +378,8 @@ an interface unit are:
> CONFIG_PPP_FILTER option is enabled, the set of packets which reset
> the transmit and receive idle timers is restricted to those which
> pass the `active' packet filter.
> + Two versions of this command exist, to deal with user space
> + expecting times as either 32-bit or 64-bit time_t seconds.
>
> * PPPIOCSMAXCID sets the maximum connection-ID parameter (and thus the
> number of connection slots) for the TCP header compressor and
> diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
> index a7b275ea5de1..1f17126c5fa4 100644
> --- a/drivers/isdn/i4l/isdn_ppp.c
> +++ b/drivers/isdn/i4l/isdn_ppp.c
> @@ -543,11 +543,19 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
> }
> is->pppcfg = val;
> break;
> - case PPPIOCGIDLE: /* get idle time information */
> + case PPPIOCGIDLE32: /* get idle time information */
> if (lp) {
> - struct ppp_idle pidle;
> + struct ppp_idle32 pidle;
> pidle.xmit_idle = pidle.recv_idle = lp->huptimer;
> - if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle))))
> + if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle32))))
> + return r;
> + }
> + break;
> + case PPPIOCGIDLE64: /* get idle time information */
> + if (lp) {
> + struct ppp_idle64 pidle;
> + pidle.xmit_idle = pidle.recv_idle = lp->huptimer;
> + if ((r = set_arg(argp, &pidle, sizeof(struct ppp_idle64))))
> return r;
> }
> break;
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 3a7aa2eed415..c8b8aa071140 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -619,7 +619,8 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> struct ppp_file *pf;
> struct ppp *ppp;
> int err = -EFAULT, val, val2, i;
> - struct ppp_idle idle;
> + struct ppp_idle32 idle32;
> + struct ppp_idle64 idle64;
> struct npioctl npi;
> int unit, cflags;
> struct slcompress *vj;
> @@ -743,10 +744,17 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> err = 0;
> break;
>
> - case PPPIOCGIDLE:
> - idle.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> - idle.recv_idle = (jiffies - ppp->last_recv) / HZ;
> - if (copy_to_user(argp, &idle, sizeof(idle)))
> + case PPPIOCGIDLE32:
> + idle32.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> + idle32.recv_idle = (jiffies - ppp->last_recv) / HZ;
> + if (copy_to_user(argp, &idle32, sizeof(idle32)))
>
Missing 'break;'
> + err = 0;
> + break;
> +
> + case PPPIOCGIDLE64:
> + idle64.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> + idle64.recv_idle = (jiffies - ppp->last_recv) / HZ;
> + if (copy_to_user(argp, &idle32, sizeof(idle32)))
>
I guess you meant 'idle64' instead of 'idle32'.
On Thu, Aug 30, 2018 at 1:06 PM Guillaume Nault <[email protected]> wrote:
> On Wed, Aug 29, 2018 at 04:03:30PM +0200, Arnd Bergmann wrote:
> > @@ -743,10 +744,17 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > err = 0;
> > break;
> >
> > - case PPPIOCGIDLE:
> > - idle.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> > - idle.recv_idle = (jiffies - ppp->last_recv) / HZ;
> > - if (copy_to_user(argp, &idle, sizeof(idle)))
> > + case PPPIOCGIDLE32:
> > + idle32.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> > + idle32.recv_idle = (jiffies - ppp->last_recv) / HZ;
> > + if (copy_to_user(argp, &idle32, sizeof(idle32)))
> >
> Missing 'break;'
>
> > + err = 0;
> > + break;
> > +
> > + case PPPIOCGIDLE64:
> > + idle64.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
> > + idle64.recv_idle = (jiffies - ppp->last_recv) / HZ;
> > + if (copy_to_user(argp, &idle32, sizeof(idle32)))
> >
> I guess you meant 'idle64' instead of 'idle32'.
Good catch, fixing up both now.
Thanks for the review!
Arnd
On Thu, Aug 30, 2018 at 1:04 PM Guillaume Nault <[email protected]> wrote:
>
> On Wed, Aug 29, 2018 at 04:03:26PM +0200, Arnd Bergmann wrote:
> > Support for handling the PPPOEIOCSFWD ioctl in compat mode was added in
> > linux-2.5.69 along with hundreds of other commands, but was always broken
> > sincen only the structure is compatible, but the command number is not,
> > due to the size being sizeof(size_t), or at first sizeof(sizeof((struct
> > sockaddr_pppox)), which is different on 64-bit architectures.
> >
> And the implementation was broken until 2016 (see 29e73269aa4d ("pppoe:
> fix reference counting in PPPoE proxy")), and nobody ever noticed. I
> should probably have removed this ioctl entirely instead of fixing it.
> Clearly, it has never been used.
>
> If you think it's worth fixing (as opposed to dropping this ioctl or
> its compat mode), then,
> Acked-by: Guillaume Nault <[email protected]>
I don't care much, but fixing it seems seems easier than coming
up with a convincing rationale for dropping.
I'll update the changelog text to include your additional background
information though, unless someone else prefers to have it dropped.
Arnd
On Thu, Aug 30, 2018 at 01:54:48PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 30, 2018 at 1:04 PM Guillaume Nault <[email protected]> wrote:
> >
> > On Wed, Aug 29, 2018 at 04:03:26PM +0200, Arnd Bergmann wrote:
> > > Support for handling the PPPOEIOCSFWD ioctl in compat mode was added in
> > > linux-2.5.69 along with hundreds of other commands, but was always broken
> > > sincen only the structure is compatible, but the command number is not,
> > > due to the size being sizeof(size_t), or at first sizeof(sizeof((struct
> > > sockaddr_pppox)), which is different on 64-bit architectures.
> > >
> > And the implementation was broken until 2016 (see 29e73269aa4d ("pppoe:
> > fix reference counting in PPPoE proxy")), and nobody ever noticed. I
> > should probably have removed this ioctl entirely instead of fixing it.
> > Clearly, it has never been used.
> >
> > If you think it's worth fixing (as opposed to dropping this ioctl or
> > its compat mode), then,
> > Acked-by: Guillaume Nault <[email protected]>
>
> I don't care much, but fixing it seems seems easier than coming
> up with a convincing rationale for dropping.
>
> I'll update the changelog text to include your additional background
> information though, unless someone else prefers to have it dropped.
>
Sounds good. Thanks.