Re: [PATCH 0/8] Rework KERN_

Andrew Morton
2012-06-05T23:09:37Z

On Wed, 06 Jun 2012 00:55:00 +0200
Kay Sievers <kay@vrfy.org> wrote:

> On Tue, 2012-06-05 at 14:28 -0700, Andrew Morton wrote:
>
> > devkmsg_writev() does weird and wonderful things with
> > facilities/levels. That function incorrectly returns "success" when
> > copy_from_user() faults, btw.
>
> Oh. Better?
>
> Thanks,
> Kay
>
>
> From: Kay Sievers <kay@vrfy.org>
> Subject: kmsg: /dev/kmsg - properly return possible copy_from_user() failure
>
> Reported-By: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Kay Sievers <kay@vrfy.org
> ---
> printk.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 32462d2..6bdacab 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -365,8 +365,10 @@ static ssize_t devkmsg_writev(struct kiocb *iocb, const struct iovec *iv,
>
> line = buf;
> for (i = 0; i < count; i++) {
> - if (copy_from_user(line, iv[i].iov_base, iv[i].iov_len))
> + if (copy_from_user(line, iv[i].iov_base, iv[i].iov_len)) {
> + ret = -EFAULT;
> goto out;
> + }
> line += iv[i].iov_len;
> }

Strictly speaking, when write() encounters an error it should return
number-of-bytes-written, or an errno if it wrote zero bytes. So
something like

--- a/kernel/printk.c~a
+++ a/kernel/printk.c
@@ -355,7 +355,7 @@ static ssize_t devkmsg_writev(struct kio
int level = default_message_loglevel;
int facility = 1; /* LOG_USER */
size_t len = iov_length(iv, count);
- ssize_t ret = len;
+ ssize_t ret;

if (len > LOG_LINE_MAX)
return -EINVAL;
@@ -365,8 +365,12 @@ static ssize_t devkmsg_writev(struct kio

line = buf;
for (i = 0; i < count; i++) {
- if (copy_from_user(line, iv[i].iov_base, iv[i].iov_len))
+ if (copy_from_user(line, iv[i].iov_base, iv[i].iov_len)) {
+ ret = line - buf;
+ if (!ret)
+ ret = -EFAULT;
goto out;
+ }
line += iv[i].iov_len;
}

@@ -396,6 +400,7 @@ static ssize_t devkmsg_writev(struct kio
line[len] = '\0';

printk_emit(facility, level, NULL, 0, "%s", line);
+ ret = 0;
out:
kfree(buf);
return ret;
_