Re: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid pointers for %pD
On Fri 2019-08-09 10:56:04, Justin He (Arm Technology China) wrote:
>
>
> > -Original Message-
> > From: Andy Shevchenko
> > Sent: 2019年8月9日 18:52
> > To: Justin He (Arm Technology China)
> > Cc: Petr Mladek ; Andy Shevchenko
> > ; Sergey Senozhatsky
> > ; Geert Uytterhoeven
> > ; Linux Kernel Mailing List > ker...@vger.kernel.org>; Thomas Gleixner ; Steven
> > Rostedt (VMware) ; Kees Cook
> > ; Shuah Khan ; Tobin C.
> > Harding
> > Subject: Re: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid
> > pointers for %pD
> >
> > On Fri, Aug 9, 2019 at 4:28 AM Jia He wrote:
> > >
> > > Commit 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing
> > invalid
> > > pointers") prevents most crash except for %pD.
> > > There is an additional pointer dereferencing before dentry_name.
> > >
> > > At least, vma->file can be NULL and be passed to printk %pD in
> > > print_bad_pte, which can cause crash.
> > >
> > > This patch fixes it with introducing a new file_dentry_name.
> > >
> >
> > Reviewed-by: Andy Shevchenko
> >
> > Perhaps you need to add a Fixes tag
> Thanks, Andy
> Fixes: 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid
> pointers")
I have added the Fixes tag and pushed the patch into printk.git,
branch for-5.4.
Thanks for the fix.
Best Regards,
Petr
Re: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid pointers for %pD
On (08/09/19 09:24), Jia He wrote:
> Commit 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid
> pointers") prevents most crash except for %pD.
> There is an additional pointer dereferencing before dentry_name.
>
> At least, vma->file can be NULL and be passed to printk %pD in
> print_bad_pte, which can cause crash.
>
> This patch fixes it with introducing a new file_dentry_name.
>
> Signed-off-by: Jia He
The series looks OK to me.
FWIW,
Reviewed-by: Sergey Senozhatsky
-ss
RE: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid pointers for %pD
> -Original Message-
> From: Andy Shevchenko
> Sent: 2019年8月9日 18:52
> To: Justin He (Arm Technology China)
> Cc: Petr Mladek ; Andy Shevchenko
> ; Sergey Senozhatsky
> ; Geert Uytterhoeven
> ; Linux Kernel Mailing List ker...@vger.kernel.org>; Thomas Gleixner ; Steven
> Rostedt (VMware) ; Kees Cook
> ; Shuah Khan ; Tobin C.
> Harding
> Subject: Re: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid
> pointers for %pD
>
> On Fri, Aug 9, 2019 at 4:28 AM Jia He wrote:
> >
> > Commit 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing
> invalid
> > pointers") prevents most crash except for %pD.
> > There is an additional pointer dereferencing before dentry_name.
> >
> > At least, vma->file can be NULL and be passed to printk %pD in
> > print_bad_pte, which can cause crash.
> >
> > This patch fixes it with introducing a new file_dentry_name.
> >
>
> Reviewed-by: Andy Shevchenko
>
> Perhaps you need to add a Fixes tag
Thanks, Andy
Fixes: 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid
pointers")
Need I reposted v2?
--
Cheers,
Justin (Jia He)
>
> > Signed-off-by: Jia He
> > ---
> > lib/vsprintf.c | 13 ++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 63937044c57d..b4a119176fdb 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -869,6 +869,15 @@ char *dentry_name(char *buf, char *end, const
> struct dentry *d, struct printf_sp
> > return widen_string(buf, n, end, spec);
> > }
> >
> > +static noinline_for_stack
> > +char *file_dentry_name(char *buf, char *end, const struct file *f,
> > + struct printf_spec spec, const char *fmt)
> > +{
> > + if (check_pointer(, end, f, spec))
> > + return buf;
> > +
> > + return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
> > +}
> > #ifdef CONFIG_BLOCK
> > static noinline_for_stack
> > char *bdev_name(char *buf, char *end, struct block_device *bdev,
> > @@ -2166,9 +2175,7 @@ char *pointer(const char *fmt, char *buf, char
> *end, void *ptr,
> > case 'C':
> > return clock(buf, end, ptr, spec, fmt);
> > case 'D':
> > - return dentry_name(buf, end,
> > - ((const struct file
> > *)ptr)->f_path.dentry,
> > - spec, fmt);
> > + return file_dentry_name(buf, end, ptr, spec, fmt);
> > #ifdef CONFIG_BLOCK
> > case 'g':
> > return bdev_name(buf, end, ptr, spec, fmt);
> > --
> > 2.17.1
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
Re: [PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid pointers for %pD
On Fri, Aug 9, 2019 at 4:28 AM Jia He wrote:
>
> Commit 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid
> pointers") prevents most crash except for %pD.
> There is an additional pointer dereferencing before dentry_name.
>
> At least, vma->file can be NULL and be passed to printk %pD in
> print_bad_pte, which can cause crash.
>
> This patch fixes it with introducing a new file_dentry_name.
>
Reviewed-by: Andy Shevchenko
Perhaps you need to add a Fixes tag
> Signed-off-by: Jia He
> ---
> lib/vsprintf.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 63937044c57d..b4a119176fdb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -869,6 +869,15 @@ char *dentry_name(char *buf, char *end, const struct
> dentry *d, struct printf_sp
> return widen_string(buf, n, end, spec);
> }
>
> +static noinline_for_stack
> +char *file_dentry_name(char *buf, char *end, const struct file *f,
> + struct printf_spec spec, const char *fmt)
> +{
> + if (check_pointer(, end, f, spec))
> + return buf;
> +
> + return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
> +}
> #ifdef CONFIG_BLOCK
> static noinline_for_stack
> char *bdev_name(char *buf, char *end, struct block_device *bdev,
> @@ -2166,9 +2175,7 @@ char *pointer(const char *fmt, char *buf, char *end,
> void *ptr,
> case 'C':
> return clock(buf, end, ptr, spec, fmt);
> case 'D':
> - return dentry_name(buf, end,
> - ((const struct file *)ptr)->f_path.dentry,
> - spec, fmt);
> + return file_dentry_name(buf, end, ptr, spec, fmt);
> #ifdef CONFIG_BLOCK
> case 'g':
> return bdev_name(buf, end, ptr, spec, fmt);
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
[PATCH 1/2] vsprintf: Prevent crash when dereferencing invalid pointers for %pD
Commit 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid
pointers") prevents most crash except for %pD.
There is an additional pointer dereferencing before dentry_name.
At least, vma->file can be NULL and be passed to printk %pD in
print_bad_pte, which can cause crash.
This patch fixes it with introducing a new file_dentry_name.
Signed-off-by: Jia He
---
lib/vsprintf.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 63937044c57d..b4a119176fdb 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -869,6 +869,15 @@ char *dentry_name(char *buf, char *end, const struct
dentry *d, struct printf_sp
return widen_string(buf, n, end, spec);
}
+static noinline_for_stack
+char *file_dentry_name(char *buf, char *end, const struct file *f,
+ struct printf_spec spec, const char *fmt)
+{
+ if (check_pointer(, end, f, spec))
+ return buf;
+
+ return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
+}
#ifdef CONFIG_BLOCK
static noinline_for_stack
char *bdev_name(char *buf, char *end, struct block_device *bdev,
@@ -2166,9 +2175,7 @@ char *pointer(const char *fmt, char *buf, char *end, void
*ptr,
case 'C':
return clock(buf, end, ptr, spec, fmt);
case 'D':
- return dentry_name(buf, end,
- ((const struct file *)ptr)->f_path.dentry,
- spec, fmt);
+ return file_dentry_name(buf, end, ptr, spec, fmt);
#ifdef CONFIG_BLOCK
case 'g':
return bdev_name(buf, end, ptr, spec, fmt);
--
2.17.1
