Kernel unaligned access at ... btrfs_real_readdir+0x51c/0x718 [btrfs]
diff mbox

Message ID 20180416191533.GZ21272@twin.jikos.cz
State New
Headers show

Commit Message

David Sterba April 16, 2018, 7:15 p.m. UTC
On Mon, Apr 16, 2018 at 09:55:45PM +0200, René Rebe wrote:
> Hi,
> 
> On 04/16/2018 06:48 PM, David Sterba wrote:
> > The warnings are valid, there's unaligned access introduced by patch
> > 
> > 23b5ec74943f44378b68c0edd8e210a86318ea5e
> > btrfs: fix readdir deadlock with pagefault
> > 
> > The directory entries (struct dir_entry) are copied to a temporary
> > buffer as they fit, ie. no alignment, and the members accessed in
> > several places.
> > 
> > The following patch adds the proper unaligned access, only compile-tested.
> > Please test and let me know, thanks!
> Would have loved to immediately give it a try, however, sorry,
> I forgot to mention I'm on the latest stable release -4.16.2-
> on a first glance this does not look like it does just apply.
> 
> I would re-base myself if I would not also have a glibc initialization 
> bug to hunt and debug, too :-/
> 
> If you happen to also rebase it for current -stable, ... ;-)

Sure, attached a 4.16.2 version.
From 4df58593a5a42c632f1c18ced3d6fae2196e29a9 Mon Sep 17 00:00:00 2001
From: David Sterba <dsterba@suse.com>
Date: Mon, 16 Apr 2018 21:10:14 +0200
Subject: [PATCH] test readdir unaligned access

---
 fs/btrfs/inode.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Rene Rebe April 17, 2018, 10:18 a.m. UTC | #1
Hi,

On 16 Apr 2018, at 21:15, David Sterba <dsterba@suse.cz> wrote:

> On Mon, Apr 16, 2018 at 09:55:45PM +0200, René Rebe wrote:
>> Hi,
>> 
>> On 04/16/2018 06:48 PM, David Sterba wrote:
>>> The warnings are valid, there's unaligned access introduced by patch
>>> 
>>> 23b5ec74943f44378b68c0edd8e210a86318ea5e
>>> btrfs: fix readdir deadlock with pagefault
>>> 
>>> The directory entries (struct dir_entry) are copied to a temporary
>>> buffer as they fit, ie. no alignment, and the members accessed in
>>> several places.
>>> 
>>> The following patch adds the proper unaligned access, only compile-tested.
>>> Please test and let me know, thanks!
>> Would have loved to immediately give it a try, however, sorry,
>> I forgot to mention I'm on the latest stable release -4.16.2-
>> on a first glance this does not look like it does just apply.
>> 
>> I would re-base myself if I would not also have a glibc initialization 
>> bug to hunt and debug, too :-/
>> 
>> If you happen to also rebase it for current -stable, ... ;-)
> 
> Sure, attached a 4.16.2 version.
> <0001-test-readdir-unaligned-access.patch>

Cool, thanks, built and so far it works, the warnings are gone.

	https://www.youtube.com/watch?v=XYsKct4T2xk

Greetings form Berlin,
	René
David Sterba April 18, 2018, 1:03 p.m. UTC | #2
On Tue, Apr 17, 2018 at 12:18:45PM +0200, René Rebe wrote:
> On 16 Apr 2018, at 21:15, David Sterba <dsterba@suse.cz> wrote:
> > On Mon, Apr 16, 2018 at 09:55:45PM +0200, René Rebe wrote:
> >> On 04/16/2018 06:48 PM, David Sterba wrote:
> >>> The warnings are valid, there's unaligned access introduced by patch
> >>> 
> >>> 23b5ec74943f44378b68c0edd8e210a86318ea5e
> >>> btrfs: fix readdir deadlock with pagefault
> >>> 
> >>> The directory entries (struct dir_entry) are copied to a temporary
> >>> buffer as they fit, ie. no alignment, and the members accessed in
> >>> several places.
> >>> 
> >>> The following patch adds the proper unaligned access, only compile-tested.
> >>> Please test and let me know, thanks!
> >> Would have loved to immediately give it a try, however, sorry,
> >> I forgot to mention I'm on the latest stable release -4.16.2-
> >> on a first glance this does not look like it does just apply.
> >> 
> >> I would re-base myself if I would not also have a glibc initialization 
> >> bug to hunt and debug, too :-/
> >> 
> >> If you happen to also rebase it for current -stable, ... ;-)
> > 
> > Sure, attached a 4.16.2 version.
> > <0001-test-readdir-unaligned-access.patch>
> 
> Cool, thanks, built and so far it works, the warnings are gone.
> 
> 	https://www.youtube.com/watch?v=XYsKct4T2xk

Thanks for testing on a real hardware! I'm not aware of regular runtime
testing on sparc, the other architectures with strict alignment
requirement have probably the emulation turned on by default so the
warnings do not appear. Or nobody bothers to report them though the
fixes are typically simple.

I like your vintage hw videos, keep going!
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c7b75dd58fad..c2df7b158820 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -44,6 +44,7 @@ 
 #include <linux/uio.h>
 #include <linux/magic.h>
 #include <linux/iversion.h>
+#include <asm/unaligned.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -5951,11 +5952,13 @@  static int btrfs_filldir(void *addr, int entries, struct dir_context *ctx)
 		struct dir_entry *entry = addr;
 		char *name = (char *)(entry + 1);
 
-		ctx->pos = entry->offset;
-		if (!dir_emit(ctx, name, entry->name_len, entry->ino,
-			      entry->type))
+		ctx->pos = get_unaligned(&entry->offset);
+		if (!dir_emit(ctx, name, get_unaligned(&entry->name_len),
+					get_unaligned(&entry->ino),
+					get_unaligned(&entry->type)))
 			return 1;
-		addr += sizeof(struct dir_entry) + entry->name_len;
+		addr += sizeof(struct dir_entry) +
+			get_unaligned(&entry->name_len);
 		ctx->pos++;
 	}
 	return 0;
@@ -6045,14 +6048,15 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		entry = addr;
-		entry->name_len = name_len;
+		put_unaligned(name_len, &entry->name_len);
 		name_ptr = (char *)(entry + 1);
 		read_extent_buffer(leaf, name_ptr, (unsigned long)(di + 1),
 				   name_len);
-		entry->type = btrfs_filetype_table[btrfs_dir_type(leaf, di)];
+		put_unaligned(btrfs_filetype_table[btrfs_dir_type(leaf, di)],
+				&entry->type);
 		btrfs_dir_item_key_to_cpu(leaf, di, &location);
-		entry->ino = location.objectid;
-		entry->offset = found_key.offset;
+		put_unaligned(location.objectid, &entry->ino);
+		put_unaligned(found_key.offset,&entry->offset);
 		entries++;
 		addr += sizeof(struct dir_entry) + name_len;
 		total_len += sizeof(struct dir_entry) + name_len;