Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1244442 > unrolled thread

Re: [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

Started byIngo Molnar <mingo@kernel.org>
First post2015-10-12 09:40 +0200
Last post2015-10-12 10:00 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in  strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Ingo Molnar <mingo@kernel.org> - 2015-10-12 09:40 +0200
    Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 "Huang\, Ying" <ying.huang@linux.intel.com> - 2015-10-12 09:50 +0200
      Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Ingo Molnar <mingo@kernel.org> - 2015-10-12 10:20 +0200
    Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Fengguang Wu <fengguang.wu@intel.com> - 2015-10-12 10:00 +0200
      Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Ingo Molnar <mingo@kernel.org> - 2015-10-12 10:20 +0200
        Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Fengguang Wu <fengguang.wu@intel.com> - 2015-10-12 10:40 +0200
      Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Fengguang Wu <fengguang.wu@intel.com> - 2015-10-12 10:30 +0200
    Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds  access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0 Fengguang Wu <fengguang.wu@intel.com> - 2015-10-12 10:00 +0200

#1244442 — Re: [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromIngo Molnar <mingo@kernel.org>
Date2015-10-12 09:40 +0200
SubjectRe: [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiFSa-Bb-11@gated-at.bofh.it>
* kernel test robot <ying.huang@linux.intel.com> wrote:

> FYI, we noticed the below changes on
> 
> git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")

Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?

I am guessing that you rebased the attached WIP commit I have in -tip (not 
permanently committed), which bases strlcpy() off strscpy() and through which 
strscpy() gains a couple of hundred usage sites:

+size_t strlcpy(char *dst, const char *src, size_t dst_size)
+{
+	int ret = strscpy(dst, src, dst_size);
+
+	/* Handle the insane and broken strlcpy() overflow return value: */
+	if (ret < 0)
+		return dst_size + strlen(src+dst_size);
+
+	return ret;
+}
+EXPORT_SYMBOL(strlcpy);

Now depending on what tree you tested it on, this KASAN report might either be a 
known and meanwhile strscpy() bug - or might perhaps be something new!

The old, known fix is:

  990486c8af04 strscpy: zero any trailing garbage bytes in the destination

> [   22.205482] systemd[1]: RTC configured in localtime, applying delta of 480 minutes to system time.
> [   22.214569] random: systemd urandom read with 11 bits of entropy available
> [   22.241378] ==================================================================
> [   22.242067] BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
> [   22.242067] Read of size 8 by task systemd/1
> [   22.242067] =============================================================================
> [   22.242067] BUG kmalloc-64 (Not tainted): kasan: bad access detected
> [   22.242067] -----------------------------------------------------------------------------
> [   22.242067] 
> [   22.242067] Disabling lock debugging due to kernel taint
> [   22.242067] INFO: Slab 0xffffea0004699980 objects=64 used=64 fp=0x          (null) flags=0x200000000000080
> [   22.242067] INFO: Object 0xffff88011a666ec0 @offset=3776 fp=0x7379732f62696c2f
> [   22.242067] 
> [   22.242067] Bytes b4 ffff88011a666eb0: 00 00 00 00 00 00 00 00 a7 4b c2 ef 07 00 00 00  .........K......
> [   22.242067] Object ffff88011a666ec0: 2f 6c 69 62 2f 73 79 73 74 65 6d 64 2f 73 79 73  /lib/systemd/sys

Is there any stack trace of this bad access?

The lack of stack trace and the unknown commit ID make it really hard to analyze 
this bug.

Thanks,

	Ingo

====================>
From 53ef1538dfe8d9ed57676c567efd0d551d0a3255 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@kernel.org>
Date: Mon, 5 Oct 2015 10:56:50 +0200
Subject: [PATCH] string: Improve the generic strlcpy() implementation

The current strlcpy() implementation has two implementational
weaknesses:

1)

There's a (largely theoretical) race:

size_t strlcpy(char *dest, const char *src, size_t size)
{
        size_t ret = strlen(src);

        if (size) {
                size_t len = (ret >= size) ? size - 1 : ret;
                memcpy(dest, src, len);
                dest[len] = '\0';
        }
        return ret;
}

If another CPU or an interrupt changes the source string after the strlen(), but
before the copy is complete, and shortens the source string, then we copy over the
NUL byte of the source buffer - including fragments of earlier source string
tails. The target buffer will still be properly NUL terminated - but it will be a
shorter string than the returned 'ret' source buffer length. (despite there not
being truncation.)

The s390 arch implementation has the same race AFAICS.

This may cause bugs if the return code is subsequently used to assume that it is
equal to the destination string's length. (While in reality it's shorter.)

The race is not automatically lethal, because it's guaranteed that the returned
length is indeed zero-delimited (due to the overlong copy we did) - so if the
string is memcpy()-ed, then it will still result in a weirdly padded but valid
string.

But if any subsequent use of the return code relies on the return code being equal
to a subsequent call of strlen(dest), then that use might lead to bugs. I.e. our
implementation of strlcpy() is indeed racy and unrobust.

But we can fix this race: by basing strlcpy() on the newly introduced strscpy()
API we iterate over the string in a single go and determine the length and
copy the string at once. Like strscpy(), but with strlcpy() return semantics.

This also makes strlcpy() faster.

2)

Another problem is that strlcpy() will also happily do bad stuff if we pass
it a negative size. Instead of that we will from now on print a (one time)
warning (by virtue of strscpy()'s overflow checking) and return.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 lib/string.c | 51 +++++++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index 96970f8a04eb..15f41de4a1b3 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -124,32 +124,6 @@ char *strncpy(char *dest, const char *src, size_t count)
 EXPORT_SYMBOL(strncpy);
 #endif
 
-#ifndef __HAVE_ARCH_STRLCPY
-/**
- * strlcpy - Copy a C-string into a sized buffer
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- * @size: size of destination buffer
- *
- * Compatible with *BSD: the result is always a valid
- * NUL-terminated string that fits in the buffer (unless,
- * of course, the buffer size is zero). It does not pad
- * out the result like strncpy() does.
- */
-size_t strlcpy(char *dest, const char *src, size_t size)
-{
-	size_t ret = strlen(src);
-
-	if (size) {
-		size_t len = (ret >= size) ? size - 1 : ret;
-		memcpy(dest, src, len);
-		dest[len] = '\0';
-	}
-	return ret;
-}
-EXPORT_SYMBOL(strlcpy);
-#endif
-
 #ifndef __HAVE_ARCH_STRSCPY
 /**
  * strscpy - Copy a C-string into a sized buffer
@@ -235,6 +209,31 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
 EXPORT_SYMBOL(strscpy);
 #endif
 
+#ifndef __HAVE_ARCH_STRLCPY
+/**
+ * strlcpy - Copy a C-string into a sized buffer
+ * @dst: Where to copy the string to
+ * @src: Where to copy the string from
+ * @dst_size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero). It does not pad
+ * out the result like strncpy() does.
+ */
+size_t strlcpy(char *dst, const char *src, size_t dst_size)
+{
+	int ret = strscpy(dst, src, dst_size);
+
+	/* Handle the insane and broken strlcpy() overflow return value: */
+	if (ret < 0)
+		return dst_size + strlen(src+dst_size);
+
+	return ret;
+}
+EXPORT_SYMBOL(strlcpy);
+#endif
+
 #ifndef __HAVE_ARCH_STRCAT
 /**
  * strcat - Append one %NUL-terminated string to another
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1244449 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

From"Huang\, Ying" <ying.huang@linux.intel.com>
Date2015-10-12 09:50 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiG1P-MT-5@gated-at.bofh.it>
In reply to#1244442
Ingo Molnar <mingo@kernel.org> writes:

> * kernel test robot <ying.huang@linux.intel.com> wrote:
>
>> FYI, we noticed the below changes on
>> 
>> git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
>> commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
>
> Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?

The test is for patch from LKML instead of git tree.  That is, you patch
is tested via applying it to a -rc kernel.

Do you have a commit in your tree for this?  We can test that to confirm.

> I am guessing that you rebased the attached WIP commit I have in -tip (not 
> permanently committed), which bases strlcpy() off strscpy() and through which 
> strscpy() gains a couple of hundred usage sites:
>
> +size_t strlcpy(char *dst, const char *src, size_t dst_size)
> +{
> +	int ret = strscpy(dst, src, dst_size);
> +
> +	/* Handle the insane and broken strlcpy() overflow return value: */
> +	if (ret < 0)
> +		return dst_size + strlen(src+dst_size);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(strlcpy);
>
> Now depending on what tree you tested it on, this KASAN report might either be a 
> known and meanwhile strscpy() bug - or might perhaps be something new!
>
> The old, known fix is:
>
>   990486c8af04 strscpy: zero any trailing garbage bytes in the destination
>
>> [   22.205482] systemd[1]: RTC configured in localtime, applying delta of 480 minutes to system time.
>> [   22.214569] random: systemd urandom read with 11 bits of entropy available
>> [   22.241378] ==================================================================
>> [   22.242067] BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
>> [   22.242067] Read of size 8 by task systemd/1
>> [   22.242067] =============================================================================
>> [   22.242067] BUG kmalloc-64 (Not tainted): kasan: bad access detected
>> [   22.242067] -----------------------------------------------------------------------------
>> [   22.242067] 
>> [   22.242067] Disabling lock debugging due to kernel taint
>> [   22.242067] INFO: Slab 0xffffea0004699980 objects=64 used=64 fp=0x          (null) flags=0x200000000000080
>> [   22.242067] INFO: Object 0xffff88011a666ec0 @offset=3776 fp=0x7379732f62696c2f
>> [   22.242067] 
>> [   22.242067] Bytes b4 ffff88011a666eb0: 00 00 00 00 00 00 00 00 a7 4b c2 ef 07 00 00 00  .........K......
>> [   22.242067] Object ffff88011a666ec0: 2f 6c 69 62 2f 73 79 73 74 65 6d 64 2f 73 79 73  /lib/systemd/sys
>
> Is there any stack trace of this bad access?

There is dmesg file attached in the original report email.  The stack
trace is as follow,

[   22.242067] Call Trace:
[   22.242067]  [<ffffffff8176e231>] dump_stack+0x4e/0x7d
[   22.242067]  [<ffffffff81203c18>] print_trailer+0xf8/0x150
[   22.242067]  [<ffffffff812068c1>] object_err+0x31/0x40
[   22.242067]  [<ffffffff8120a6e5>] kasan_report_error+0x1e5/0x3f0
[   22.242067]  [<ffffffff811d9c63>] ? anon_vma_interval_tree_insert+0x123/0x140
[   22.242067]  [<ffffffff8120a9d4>] kasan_report+0x34/0x40
[   22.242067]  [<ffffffff8177a3e8>] ? strlcpy+0xc8/0x250
[   22.242067]  [<ffffffff81209ed4>] __asan_load8+0x64/0xa0
[   22.242067]  [<ffffffff8177a3e8>] strlcpy+0xc8/0x250
[   22.242067]  [<ffffffff8117b1b7>] cgroup_release_agent_write+0x67/0xa0
[   22.242067]  [<ffffffff81179925>] cgroup_file_write+0x75/0x180
[   22.242067]  [<ffffffff811798b0>] ? cgroup_init_cftypes+0x160/0x160
[   22.242067]  [<ffffffff812af81e>] kernfs_fop_write+0x17e/0x210
[   22.242067]  [<ffffffff8121cf67>] __vfs_write+0x57/0x170
[   22.242067]  [<ffffffff81117b73>] ? preempt_count_sub+0x13/0xe0
[   22.242067]  [<ffffffff8113c211>] ? update_fast_ctr+0x51/0x80
[   22.242067]  [<ffffffff8121d2bb>] vfs_write+0xeb/0x240
[   22.242067]  [<ffffffff8121d513>] SyS_write+0x53/0xb0
[   22.242067]  [<ffffffff8242f276>] entry_SYSCALL_64_fastpath+0x16/0x75

Best Regards,
Huang, Ying


> The lack of stack trace and the unknown commit ID make it really hard to analyze 
> this bug.
>
> Thanks,
>
> 	Ingo
>
> ====================>
> From 53ef1538dfe8d9ed57676c567efd0d551d0a3255 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <mingo@kernel.org>
> Date: Mon, 5 Oct 2015 10:56:50 +0200
> Subject: [PATCH] string: Improve the generic strlcpy() implementation
>
> The current strlcpy() implementation has two implementational
> weaknesses:
>
> 1)
>
> There's a (largely theoretical) race:
>
> size_t strlcpy(char *dest, const char *src, size_t size)
> {
>         size_t ret = strlen(src);
>
>         if (size) {
>                 size_t len = (ret >= size) ? size - 1 : ret;
>                 memcpy(dest, src, len);
>                 dest[len] = '\0';
>         }
>         return ret;
> }
>
> If another CPU or an interrupt changes the source string after the strlen(), but
> before the copy is complete, and shortens the source string, then we copy over the
> NUL byte of the source buffer - including fragments of earlier source string
> tails. The target buffer will still be properly NUL terminated - but it will be a
> shorter string than the returned 'ret' source buffer length. (despite there not
> being truncation.)
>
> The s390 arch implementation has the same race AFAICS.
>
> This may cause bugs if the return code is subsequently used to assume that it is
> equal to the destination string's length. (While in reality it's shorter.)
>
> The race is not automatically lethal, because it's guaranteed that the returned
> length is indeed zero-delimited (due to the overlong copy we did) - so if the
> string is memcpy()-ed, then it will still result in a weirdly padded but valid
> string.
>
> But if any subsequent use of the return code relies on the return code being equal
> to a subsequent call of strlen(dest), then that use might lead to bugs. I.e. our
> implementation of strlcpy() is indeed racy and unrobust.
>
> But we can fix this race: by basing strlcpy() on the newly introduced strscpy()
> API we iterate over the string in a single go and determine the length and
> copy the string at once. Like strscpy(), but with strlcpy() return semantics.
>
> This also makes strlcpy() faster.
>
> 2)
>
> Another problem is that strlcpy() will also happily do bad stuff if we pass
> it a negative size. Instead of that we will from now on print a (one time)
> warning (by virtue of strscpy()'s overflow checking) and return.
>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  lib/string.c | 51 +++++++++++++++++++++++++--------------------------
>  1 file changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index 96970f8a04eb..15f41de4a1b3 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -124,32 +124,6 @@ char *strncpy(char *dest, const char *src, size_t count)
>  EXPORT_SYMBOL(strncpy);
>  #endif
>  
> -#ifndef __HAVE_ARCH_STRLCPY
> -/**
> - * strlcpy - Copy a C-string into a sized buffer
> - * @dest: Where to copy the string to
> - * @src: Where to copy the string from
> - * @size: size of destination buffer
> - *
> - * Compatible with *BSD: the result is always a valid
> - * NUL-terminated string that fits in the buffer (unless,
> - * of course, the buffer size is zero). It does not pad
> - * out the result like strncpy() does.
> - */
> -size_t strlcpy(char *dest, const char *src, size_t size)
> -{
> -	size_t ret = strlen(src);
> -
> -	if (size) {
> -		size_t len = (ret >= size) ? size - 1 : ret;
> -		memcpy(dest, src, len);
> -		dest[len] = '\0';
> -	}
> -	return ret;
> -}
> -EXPORT_SYMBOL(strlcpy);
> -#endif
> -
>  #ifndef __HAVE_ARCH_STRSCPY
>  /**
>   * strscpy - Copy a C-string into a sized buffer
> @@ -235,6 +209,31 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
>  EXPORT_SYMBOL(strscpy);
>  #endif
>  
> +#ifndef __HAVE_ARCH_STRLCPY
> +/**
> + * strlcpy - Copy a C-string into a sized buffer
> + * @dst: Where to copy the string to
> + * @src: Where to copy the string from
> + * @dst_size: size of destination buffer
> + *
> + * Compatible with *BSD: the result is always a valid
> + * NUL-terminated string that fits in the buffer (unless,
> + * of course, the buffer size is zero). It does not pad
> + * out the result like strncpy() does.
> + */
> +size_t strlcpy(char *dst, const char *src, size_t dst_size)
> +{
> +	int ret = strscpy(dst, src, dst_size);
> +
> +	/* Handle the insane and broken strlcpy() overflow return value: */
> +	if (ret < 0)
> +		return dst_size + strlen(src+dst_size);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(strlcpy);
> +#endif
> +
>  #ifndef __HAVE_ARCH_STRCAT
>  /**
>   * strcat - Append one %NUL-terminated string to another
> _______________________________________________
> LKP mailing list
> LKP@lists.01.org
> https://lists.01.org/mailman/listinfo/lkp
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244476 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromIngo Molnar <mingo@kernel.org>
Date2015-10-12 10:20 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGuS-1Ag-15@gated-at.bofh.it>
In reply to#1244449
* Huang, Ying <ying.huang@linux.intel.com> wrote:

> Ingo Molnar <mingo@kernel.org> writes:
> 
> > * kernel test robot <ying.huang@linux.intel.com> wrote:
> >
> >> FYI, we noticed the below changes on
> >> 
> >> git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> >> commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> >
> > Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?
> 
> The test is for patch from LKML instead of git tree.  That is, you patch
> is tested via applying it to a -rc kernel.
> 
> Do you have a commit in your tree for this?  We can test that to confirm.

Yeah, I just made a merge that includes just to strscpy() related bits:

b94371b0917a Merge tag 'v4.3-rc5' into core/strings, to pick up strscpy() fixes

(Note, it might take a few minutes for korg git mirrors to pick up this merge.)

All that tree does is that it makes strlcpy() use strscpy():

> > +size_t strlcpy(char *dst, const char *src, size_t dst_size)
> > +{
> > +	int ret = strscpy(dst, src, dst_size);
> > +
> > +	/* Handle the insane and broken strlcpy() overflow return value: */
> > +	if (ret < 0)
> > +		return dst_size + strlen(src+dst_size);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(strlcpy);

Do you see the same KASAN failure with that commit? If my analysis below is 
correct, then the failure should go away.

Analysis:

The stack dump:

[   22.242067]  [<ffffffff8177a3e8>] strlcpy+0xc8/0x250
[   22.242067]  [<ffffffff8117b1b7>] cgroup_release_agent_write+0x67/0xa0
[   22.242067]  [<ffffffff81179925>] cgroup_file_write+0x75/0x180
[   22.242067]  [<ffffffff812af81e>] kernfs_fop_write+0x17e/0x210
[   22.242067]  [<ffffffff8121cf67>] __vfs_write+0x57/0x170
[   22.242067]  [<ffffffff8121d2bb>] vfs_write+0xeb/0x240

Implicates this strlcpy():

        spin_lock(&release_agent_path_lock);
        strlcpy(cgrp->root->release_agent_path, strstrip(buf),
                sizeof(cgrp->root->release_agent_path));
        spin_unlock(&release_agent_path_lock);

where:

include/linux/cgroup-defs.h:    char release_agent_path[PATH_MAX];

the target buffer sizing looks pretty robust (because simple).

And the input buffer side looks safe as well, by my reading:

'buf' here seems like a regular write operation, with a 'buf' and a 'size' 
parameter - layered in through various layers of abstraction:

 struct cftype::write, used in kernel/cgroup.c: cgroup_file_write()

 - no size checks, no guarantee that we have a string

this is called via:

 struct kernfs_ops::write via kernfs

which guarantees string termination in fs/kernfs/file.c's kernfs_fop_write():

        buf[len] = '\0';        /* guarantee string termination */

        ops = kernfs_ops(of->kn);
        if (ops->write)
                len = ops->write(of, buf, len, *ppos);

and that's a stable, private string local to the calling task.

So my guess is that this is the bug that got fixed by:

  990486c8af04 ("strscpy: zero any trailing garbage bytes in the destination")

that that systemd passed in a string with leading whitespace, thus strtrim() 
created an unaligned string, which caused the strscpy() to access past the end of 
the kmalloc() buffer.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244459 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromFengguang Wu <fengguang.wu@intel.com>
Date2015-10-12 10:00 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGbx-Yi-9@gated-at.bofh.it>
In reply to#1244442
On Mon, Oct 12, 2015 at 03:51:04PM +0800, Fengguang Wu wrote:
> On Mon, Oct 12, 2015 at 09:33:55AM +0200, Ingo Molnar wrote:
> > 
> > * kernel test robot <ying.huang@linux.intel.com> wrote:
> > 
> > > FYI, we noticed the below changes on
> > > 
> > > git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > > commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> > 
> > Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?
> 
> Ingo, all applied patches will be uploaded to github from now on.
> 
> Here is the exact commit:
> 
> https://github.com/0day-ci/linux/commits/Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation

Sorry that's already the rebased commit.. The old version was applied
to 4.3-rc4 while the new one is applied to 4.3-rc5.

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244474 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromIngo Molnar <mingo@kernel.org>
Date2015-10-12 10:20 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGuR-1Ag-13@gated-at.bofh.it>
In reply to#1244459
* Fengguang Wu <fengguang.wu@intel.com> wrote:

> On Mon, Oct 12, 2015 at 03:51:04PM +0800, Fengguang Wu wrote:
> > On Mon, Oct 12, 2015 at 09:33:55AM +0200, Ingo Molnar wrote:
> > > 
> > > * kernel test robot <ying.huang@linux.intel.com> wrote:
> > > 
> > > > FYI, we noticed the below changes on
> > > > 
> > > > git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > > > commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> > > 
> > > Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?
> > 
> > Ingo, all applied patches will be uploaded to github from now on.

Thanks!

You might want to move that to korg instead, because many people don't like to 
pull from github.

> > Here is the exact commit:
> > 
> > https://github.com/0day-ci/linux/commits/Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> 
> Sorry that's already the rebased commit.. The old version was applied
> to 4.3-rc4 while the new one is applied to 4.3-rc5.

So as long as you have the tested sha1 mentioned in the bug report, and that sha1 
can be pulled from somewhere on korg, I'm a happy camper: in this particular case 
it would have told me whether your testing tree had upstream fix 990486c8af or 
not.

Rebasing and applying email patches for testing purposes is otherwise perfectly 
OK, as long as the precise Git tree used for testing can be fetched.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244488 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromFengguang Wu <fengguang.wu@intel.com>
Date2015-10-12 10:40 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGOf-1WT-21@gated-at.bofh.it>
In reply to#1244474
On Mon, Oct 12, 2015 at 10:17:14AM +0200, Ingo Molnar wrote:
> 
> * Fengguang Wu <fengguang.wu@intel.com> wrote:
> 
> > On Mon, Oct 12, 2015 at 03:51:04PM +0800, Fengguang Wu wrote:
> > > On Mon, Oct 12, 2015 at 09:33:55AM +0200, Ingo Molnar wrote:
> > > > 
> > > > * kernel test robot <ying.huang@linux.intel.com> wrote:
> > > > 
> > > > > FYI, we noticed the below changes on
> > > > > 
> > > > > git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > > > > commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> > > > 
> > > > Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?
> > > 
> > > Ingo, all applied patches will be uploaded to github from now on.
> 
> Thanks!
> 
> You might want to move that to korg instead, because many people don't like to 
> pull from github.

That'd be good, however github would match its security level better
-- it's a robot doing git upload, so the ssh private key must be kept
in a server where several team members can see it.

> > > Here is the exact commit:
> > > 
> > > https://github.com/0day-ci/linux/commits/Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > 
> > Sorry that's already the rebased commit.. The old version was applied
> > to 4.3-rc4 while the new one is applied to 4.3-rc5.
> 
> So as long as you have the tested sha1 mentioned in the bug report, and that sha1 
> can be pulled from somewhere on korg, I'm a happy camper: in this particular case 
> it would have told me whether your testing tree had upstream fix 990486c8af or 
> not.
> 
> Rebasing and applying email patches for testing purposes is otherwise perfectly 
> OK, as long as the precise Git tree used for testing can be fetched.

FYI I've just added timestamp to the branch name -- which helps make
the reported URL consistent and immutable over time.

You are nice and easy to work with, however it'll have to work well
with lots of people in all kinds of situations. :)

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244478 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromFengguang Wu <fengguang.wu@intel.com>
Date2015-10-12 10:30 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGEy-1LJ-15@gated-at.bofh.it>
In reply to#1244459
On Mon, Oct 12, 2015 at 03:58:32PM +0800, Fengguang Wu wrote:
> On Mon, Oct 12, 2015 at 03:51:04PM +0800, Fengguang Wu wrote:
> > On Mon, Oct 12, 2015 at 09:33:55AM +0200, Ingo Molnar wrote:
> > > 
> > > * kernel test robot <ying.huang@linux.intel.com> wrote:
> > > 
> > > > FYI, we noticed the below changes on
> > > > 
> > > > git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > > > commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> > > 
> > > Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?
> > 
> > Ingo, all applied patches will be uploaded to github from now on.
> > 
> > Here is the exact commit:
> > 
> > https://github.com/0day-ci/linux/commits/Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> 
> Sorry that's already the rebased commit.. The old version was applied
> to 4.3-rc4 while the new one is applied to 4.3-rc5.

I'll append a time stamp to the branch name to avoid such confusions in future.

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1244460 — Re: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0

FromFengguang Wu <fengguang.wu@intel.com>
Date2015-10-12 10:00 +0200
SubjectRe: [LKP] [lkp] [string] 5f6f0801f5: BUG: KASan: out of bounds access in strlcpy+0xc8/0x250 at addr ffff88011a666ee0
Message-ID<qiGbx-Yi-5@gated-at.bofh.it>
In reply to#1244442
On Mon, Oct 12, 2015 at 09:33:55AM +0200, Ingo Molnar wrote:
> 
> * kernel test robot <ying.huang@linux.intel.com> wrote:
> 
> > FYI, we noticed the below changes on
> > 
> > git://internal_mailing_list_patch_tree Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation
> > commit 5f6f0801f5fdfce4984c6a14f99dbfbb417acb66 ("string: Improve the generic strlcpy() implementation")
> 
> Hm, there's no such commit ID anywhere I can see - did you rebase my tree perhaps?

Ingo, all applied patches will be uploaded to github from now on.

Here is the exact commit:

https://github.com/0day-ci/linux/commits/Ingo-Molnar/string-Improve-the-generic-strlcpy-implementation

Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web