Re: [RFC v3 6/9] kmsg: add ioctl for adding and deleting kmsg* devices

[Arnd Bergmann]

On Monday 19 October 2015 14:58:20 Paul Osmialowski wrote:
> +
> +struct kmsg_cmd_buffer_add {
> +   size_t size;
> +   unsigned short mode;
> +   int minor;
> +} __attribute__((packed));
> +
> +#define KMSG_IOCTL_MAGIC   0xBB
> +
> +/*
> 

Try to avoid using packed unaligned data structures. Here I would
just use __u64 and __u32 members.

> +   case KMSG_CMD_BUFFER_ADD:
> +   if (copy_from_user(, argp, sizeof(size)))
> +   return -EFAULT;
> +   argp += sizeof(size);
> +   if (copy_from_user(, argp, sizeof(mode)))
> +   return -EFAULT;

This is a rather unusual way to access the data. Just copy the
entire structure to the stack.

Arnd
--
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/

[RFC v3 6/9] kmsg: add ioctl for adding and deleting kmsg* devices

[Paul Osmialowski]

From: Marcin Niesluchowski 

There is no possibility to add/delete kmsg* buffers from userspace.

Adds following ioctl for main kmsg device adding and deleting
additional kmsg devices:
* KMSG_CMD_BUFFER_ADD
* KMSG_CMD_BUFFER_DEL

Signed-off-by: Marcin Niesluchowski 
Signed-off-by: Paul Osmialowski 
---
 Documentation/ioctl/ioctl-number.txt |   1 +
 drivers/char/mem.c   |   2 +-
 include/linux/printk.h   |   7 ++
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/kmsg_ioctl.h  |  30 +
 kernel/printk/kmsg.c | 123 +++
 6 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/kmsg_ioctl.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 43e6923..76dec8b 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -319,6 +319,7 @@ Code  Seq#(hex) Include FileComments

 0xB1   00-1F   PPPoX   
 0xB3   00  linux/mmc/ioctl.h
+0xBB   00-02   uapi/linux/kmsg_ioctl.h
 0xC0   00-0F   linux/usb/iowarrior.h
 0xCA   00-0F   uapi/misc/cxl.h
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 7d46234..ac824de 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -808,7 +808,7 @@ static int memory_open(struct inode *inode, struct file 
*filp)
 
minor = iminor(inode);
if (minor >= ARRAY_SIZE(devlist))
-   return kmsg_memory_open(inode, filp);
+   return kmsg_memory_open_ext(inode, filp);
 
dev = [minor];
if (!dev->fops)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 35111e8..294adab 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -427,9 +427,11 @@ extern struct class *mem_class;
 #define KMSG_MINOR 11
 
 extern const struct file_operations kmsg_fops;
+extern const struct file_operations kmsg_fops_ext;
 
 extern struct device *init_kmsg(int minor, umode_t mode);
 extern int kmsg_memory_open(struct inode *inode, struct file *filp);
+extern int kmsg_memory_open_ext(struct inode *inode, struct file *filp);
 extern int kmsg_mode(int minor, umode_t *mode);
 extern int kmsg_sys_buffer_add(size_t size, umode_t mode);
 extern void kmsg_sys_buffer_del(int minor);
@@ -446,6 +448,11 @@ static inline int kmsg_memory_open(struct inode *inode, 
struct file *filp)
return -ENXIO;
 }
 
+static inline int kmsg_memory_open_ext(struct inode *inode, struct file *filp)
+{
+   return -ENXIO;
+}
+
 static inline int kmsg_mode(int minor, umode_t *mode)
 {
return -ENXIO;
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index e777078..d998999 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -225,6 +225,7 @@ header-y += kernel-page-flags.h
 header-y += kexec.h
 header-y += keyboard.h
 header-y += keyctl.h
+header-y += kmsg_ioctl.h
 
 ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm.h \
  $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h),)
diff --git a/include/uapi/linux/kmsg_ioctl.h b/include/uapi/linux/kmsg_ioctl.h
new file mode 100644
index 000..89c0c61
--- /dev/null
+++ b/include/uapi/linux/kmsg_ioctl.h
@@ -0,0 +1,30 @@
+/*
+ * This is ioctl include for kmsg* devices
+ */
+
+#ifndef _KMSG_IOCTL_H_
+#define _KMSG_IOCTL_H_
+
+#include 
+#include 
+
+struct kmsg_cmd_buffer_add {
+   size_t size;
+   unsigned short mode;
+   int minor;
+} __attribute__((packed));
+
+#define KMSG_IOCTL_MAGIC   0xBB
+
+/*
+ * A ioctl interface for kmsg device.
+ *
+ * KMSG_CMD_BUFFER_ADD:Creates additional kmsg device based on its size
+ * and mode. Minor of created device is put.
+ * KMSG_CMD_BUFFER_DEL:Removes additional kmsg device based on its 
minor
+ */
+#define KMSG_CMD_BUFFER_ADD_IOWR(KMSG_IOCTL_MAGIC, 0x00, \
+ struct kmsg_cmd_buffer_add)
+#define KMSG_CMD_BUFFER_DEL_IOW(KMSG_IOCTL_MAGIC, 0x01, int)
+
+#endif
diff --git a/kernel/printk/kmsg.c b/kernel/printk/kmsg.c
index 184575b..f91a64a 100644
--- a/kernel/printk/kmsg.c
+++ b/kernel/printk/kmsg.c
@@ -22,8 +22,12 @@
 
 #include 
 
+#include 
+
 #include "printk.h"
 
+#define KMSG_MAX_MINOR_LEN 20
+
 /* /dev/kmsg - userspace message inject/listen interface */
 struct devkmsg_user {
u64 seq;
@@ -407,6 +411,118 @@ const struct file_operations kmsg_fops = {
.release = devkmsg_release,
 };
 
+static int kmsg_open_ext(struct inode *inode, struct file *file)
+{
+   return kmsg_fops.open(inode, file);
+}
+
+static ssize_t kmsg_write_iter_ext(struct kiocb *iocb, struct iov_iter *from)
+{
+   return kmsg_fops.write_iter(iocb, from);
+}
+
+static ssize_t kmsg_read_ext(struct file *file, 

[RFC v3 6/9] kmsg: add ioctl for adding and deleting kmsg* devices

[Paul Osmialowski]

From: Marcin Niesluchowski 

There is no possibility to add/delete kmsg* buffers from userspace.

Adds following ioctl for main kmsg device adding and deleting
additional kmsg devices:
* KMSG_CMD_BUFFER_ADD
* KMSG_CMD_BUFFER_DEL

Signed-off-by: Marcin Niesluchowski 
Signed-off-by: Paul Osmialowski 
---
 Documentation/ioctl/ioctl-number.txt |   1 +
 drivers/char/mem.c   |   2 +-
 include/linux/printk.h   |   7 ++
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/kmsg_ioctl.h  |  30 +
 kernel/printk/kmsg.c | 123 +++
 6 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/kmsg_ioctl.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 43e6923..76dec8b 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -319,6 +319,7 @@ Code  Seq#(hex) Include FileComments

 0xB1   00-1F   PPPoX   
 0xB3   00  linux/mmc/ioctl.h
+0xBB   00-02   uapi/linux/kmsg_ioctl.h
 0xC0   00-0F   linux/usb/iowarrior.h
 0xCA   00-0F   uapi/misc/cxl.h
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 7d46234..ac824de 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -808,7 +808,7 @@ static int memory_open(struct inode *inode, struct file 
*filp)
 
minor = iminor(inode);
if (minor >= ARRAY_SIZE(devlist))
-   return kmsg_memory_open(inode, filp);
+   return kmsg_memory_open_ext(inode, filp);
 
dev = [minor];
if (!dev->fops)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 35111e8..294adab 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -427,9 +427,11 @@ extern struct class *mem_class;
 #define KMSG_MINOR 11
 
 extern const struct file_operations kmsg_fops;
+extern const struct file_operations kmsg_fops_ext;
 
 extern struct device *init_kmsg(int minor, umode_t mode);
 extern int kmsg_memory_open(struct inode *inode, struct file *filp);
+extern int kmsg_memory_open_ext(struct inode *inode, struct file *filp);
 extern int kmsg_mode(int minor, umode_t *mode);
 extern int kmsg_sys_buffer_add(size_t size, umode_t mode);
 extern void kmsg_sys_buffer_del(int minor);
@@ -446,6 +448,11 @@ static inline int kmsg_memory_open(struct inode *inode, 
struct file *filp)
return -ENXIO;
 }
 
+static inline int kmsg_memory_open_ext(struct inode *inode, struct file *filp)
+{
+   return -ENXIO;
+}
+
 static inline int kmsg_mode(int minor, umode_t *mode)
 {
return -ENXIO;
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index e777078..d998999 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -225,6 +225,7 @@ header-y += kernel-page-flags.h
 header-y += kexec.h
 header-y += keyboard.h
 header-y += keyctl.h
+header-y += kmsg_ioctl.h
 
 ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm.h \
  $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h),)
diff --git a/include/uapi/linux/kmsg_ioctl.h b/include/uapi/linux/kmsg_ioctl.h
new file mode 100644
index 000..89c0c61
--- /dev/null
+++ b/include/uapi/linux/kmsg_ioctl.h
@@ -0,0 +1,30 @@
+/*
+ * This is ioctl include for kmsg* devices
+ */
+
+#ifndef _KMSG_IOCTL_H_
+#define _KMSG_IOCTL_H_
+
+#include 
+#include 
+
+struct kmsg_cmd_buffer_add {
+   size_t size;
+   unsigned short mode;
+   int minor;
+} __attribute__((packed));
+
+#define KMSG_IOCTL_MAGIC   0xBB
+
+/*
+ * A ioctl interface for kmsg device.
+ *
+ * KMSG_CMD_BUFFER_ADD:Creates additional kmsg device based on its size
+ * and mode. Minor of created device is put.
+ * KMSG_CMD_BUFFER_DEL:Removes additional kmsg device based on its 
minor
+ */
+#define KMSG_CMD_BUFFER_ADD_IOWR(KMSG_IOCTL_MAGIC, 0x00, \
+ struct kmsg_cmd_buffer_add)
+#define KMSG_CMD_BUFFER_DEL_IOW(KMSG_IOCTL_MAGIC, 0x01, int)
+
+#endif
diff --git a/kernel/printk/kmsg.c b/kernel/printk/kmsg.c
index 184575b..f91a64a 100644
--- a/kernel/printk/kmsg.c
+++ b/kernel/printk/kmsg.c
@@ -22,8 +22,12 @@
 
 #include 
 
+#include 
+
 #include "printk.h"
 
+#define KMSG_MAX_MINOR_LEN 20
+
 /* /dev/kmsg - userspace message inject/listen interface */
 struct devkmsg_user {
u64 seq;
@@ -407,6 +411,118 @@ const struct file_operations kmsg_fops = {
.release = devkmsg_release,
 };
 
+static int kmsg_open_ext(struct inode *inode, struct file *file)
+{
+   return kmsg_fops.open(inode, file);
+}
+
+static ssize_t kmsg_write_iter_ext(struct kiocb *iocb, struct iov_iter *from)
+{
+   return 

Re: [RFC v3 6/9] kmsg: add ioctl for adding and deleting kmsg* devices

[Arnd Bergmann]

On Monday 19 October 2015 14:58:20 Paul Osmialowski wrote:
> +
> +struct kmsg_cmd_buffer_add {
> +   size_t size;
> +   unsigned short mode;
> +   int minor;
> +} __attribute__((packed));
> +
> +#define KMSG_IOCTL_MAGIC   0xBB
> +
> +/*
> 

Try to avoid using packed unaligned data structures. Here I would
just use __u64 and __u32 members.

> +   case KMSG_CMD_BUFFER_ADD:
> +   if (copy_from_user(, argp, sizeof(size)))
> +   return -EFAULT;
> +   argp += sizeof(size);
> +   if (copy_from_user(, argp, sizeof(mode)))
> +   return -EFAULT;

This is a rather unusual way to access the data. Just copy the
entire structure to the stack.

Arnd
--
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/