* Bug report: unaligned access with ext4 encryption @ 2018-12-30 16:29 ` Aaro Koskinen 0 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2018-12-30 16:29 UTC (permalink / raw) To: Theodore Y. Ts'o, Jaegeuk Kim, David Howells Cc: linux-fscrypt, linux-kernel, keyrings Hi, When using ext4 encryption on SPARC, there's plenty of dmesg noise about unaligned access: [ 167.269526] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 167.270152] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.087451] log_unaligned: 5 callbacks suppressed [ 181.087511] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.092435] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.095816] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 And also seen on an ARM machine: $ cat /proc/cpu/alignment User: 0 System: 1028193 (find_and_lock_process_key+0x84/0x10c) Skipped: 0 Half: 0 Word: 1028193 DWord: 0 Multi: 0 User faults: 0 (ignored) Looks like user_key_payload layout is not optimal when data address is used for fscrypt_key... I tried the below change and got rid of the messages. Not sure what the proper fix should be? A. diff --git a/include/keys/user-type.h b/include/keys/user-type.h index e098cbe27db5..6495ffcfe510 100644 --- a/include/keys/user-type.h +++ b/include/keys/user-type.h @@ -31,7 +31,7 @@ struct user_key_payload { struct rcu_head rcu; /* RCU destructor */ unsigned short datalen; /* length of this data */ - char data[0]; /* actual data */ + char data[0] __aligned(4); /* actual data */ }; extern struct key_type key_type_user; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Bug report: unaligned access with ext4 encryption @ 2018-12-30 16:29 ` Aaro Koskinen 0 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2018-12-30 16:29 UTC (permalink / raw) To: Theodore Y. Ts'o, Jaegeuk Kim, David Howells Cc: linux-fscrypt, linux-kernel, keyrings Hi, When using ext4 encryption on SPARC, there's plenty of dmesg noise about unaligned access: [ 167.269526] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 167.270152] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.087451] log_unaligned: 5 callbacks suppressed [ 181.087511] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.092435] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 [ 181.095816] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 And also seen on an ARM machine: $ cat /proc/cpu/alignment User: 0 System: 1028193 (find_and_lock_process_key+0x84/0x10c) Skipped: 0 Half: 0 Word: 1028193 DWord: 0 Multi: 0 User faults: 0 (ignored) Looks like user_key_payload layout is not optimal when data address is used for fscrypt_key... I tried the below change and got rid of the messages. Not sure what the proper fix should be? A. diff --git a/include/keys/user-type.h b/include/keys/user-type.h index e098cbe27db5..6495ffcfe510 100644 --- a/include/keys/user-type.h +++ b/include/keys/user-type.h @@ -31,7 +31,7 @@ struct user_key_payload { struct rcu_head rcu; /* RCU destructor */ unsigned short datalen; /* length of this data */ - char data[0]; /* actual data */ + char data[0] __aligned(4); /* actual data */ }; extern struct key_type key_type_user; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption 2018-12-30 16:29 ` Aaro Koskinen @ 2019-01-03 17:17 ` Eric Biggers -1 siblings, 0 replies; 11+ messages in thread From: Eric Biggers @ 2019-01-03 17:17 UTC (permalink / raw) To: Aaro Koskinen, David Howells Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings On Sun, Dec 30, 2018 at 06:29:06PM +0200, Aaro Koskinen wrote: > Hi, > > When using ext4 encryption on SPARC, there's plenty of dmesg noise about > unaligned access: > > [ 167.269526] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 167.270152] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.087451] log_unaligned: 5 callbacks suppressed > [ 181.087511] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.092435] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.095816] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > > And also seen on an ARM machine: > > $ cat /proc/cpu/alignment > User: 0 > System: 1028193 (find_and_lock_process_key+0x84/0x10c) > Skipped: 0 > Half: 0 > Word: 1028193 > DWord: 0 > Multi: 0 > User faults: 0 (ignored) > > Looks like user_key_payload layout is not optimal when data address > is used for fscrypt_key... I tried the below change and got rid of the > messages. Not sure what the proper fix should be? > > A. > > diff --git a/include/keys/user-type.h b/include/keys/user-type.h > index e098cbe27db5..6495ffcfe510 100644 > --- a/include/keys/user-type.h > +++ b/include/keys/user-type.h > @@ -31,7 +31,7 @@ > struct user_key_payload { > struct rcu_head rcu; /* RCU destructor */ > unsigned short datalen; /* length of this data */ > - char data[0]; /* actual data */ > + char data[0] __aligned(4); /* actual data */ > }; > > extern struct key_type key_type_user; > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes much more sense to have the keyrings subsystem store the payload with better alignment, than to work around the 2-byte alignment in fscrypt. But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. David, what do you think? - Eric ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption @ 2019-01-03 17:17 ` Eric Biggers 0 siblings, 0 replies; 11+ messages in thread From: Eric Biggers @ 2019-01-03 17:17 UTC (permalink / raw) To: Aaro Koskinen, David Howells Cc: Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings On Sun, Dec 30, 2018 at 06:29:06PM +0200, Aaro Koskinen wrote: > Hi, > > When using ext4 encryption on SPARC, there's plenty of dmesg noise about > unaligned access: > > [ 167.269526] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 167.270152] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.087451] log_unaligned: 5 callbacks suppressed > [ 181.087511] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.092435] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > [ 181.095816] Kernel unaligned access at TPC[5497a0] find_and_lock_process_key+0x80/0x120 > > And also seen on an ARM machine: > > $ cat /proc/cpu/alignment > User: 0 > System: 1028193 (find_and_lock_process_key+0x84/0x10c) > Skipped: 0 > Half: 0 > Word: 1028193 > DWord: 0 > Multi: 0 > User faults: 0 (ignored) > > Looks like user_key_payload layout is not optimal when data address > is used for fscrypt_key... I tried the below change and got rid of the > messages. Not sure what the proper fix should be? > > A. > > diff --git a/include/keys/user-type.h b/include/keys/user-type.h > index e098cbe27db5..6495ffcfe510 100644 > --- a/include/keys/user-type.h > +++ b/include/keys/user-type.h > @@ -31,7 +31,7 @@ > struct user_key_payload { > struct rcu_head rcu; /* RCU destructor */ > unsigned short datalen; /* length of this data */ > - char data[0]; /* actual data */ > + char data[0] __aligned(4); /* actual data */ > }; > > extern struct key_type key_type_user; > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes much more sense to have the keyrings subsystem store the payload with better alignment, than to work around the 2-byte alignment in fscrypt. But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. David, what do you think? - Eric ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption 2019-01-03 17:17 ` Eric Biggers (?) @ 2019-01-04 17:28 ` David Howells 2019-01-10 22:29 ` Aaro Koskinen -1 siblings, 1 reply; 11+ messages in thread From: David Howells @ 2019-01-04 17:28 UTC (permalink / raw) To: Eric Biggers Cc: dhowells, Aaro Koskinen, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings Eric Biggers <ebiggers@kernel.org> wrote: > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > much more sense to have the keyrings subsystem store the payload with better > alignment, than to work around the 2-byte alignment in fscrypt. > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > David, what do you think? Does that even work? Might be better to just insert 6 bytes of padding with a comment, but yes I agree that it's probably better to align it to at least machine word size. David ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption 2019-01-04 17:28 ` David Howells @ 2019-01-10 22:29 ` Aaro Koskinen 0 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2019-01-10 22:29 UTC (permalink / raw) To: David Howells Cc: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings Hi, On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > Eric Biggers <ebiggers@kernel.org> wrote: > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > much more sense to have the keyrings subsystem store the payload with better > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > David, what do you think? > > Does that even work? That should work. > Might be better to just insert 6 bytes of padding with a comment, but yes I > agree that it's probably better to align it to at least machine word size. Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should make it always right automatically. A. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption @ 2019-01-10 22:29 ` Aaro Koskinen 0 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2019-01-10 22:29 UTC (permalink / raw) To: David Howells Cc: Eric Biggers, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings Hi, On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > Eric Biggers <ebiggers@kernel.org> wrote: > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > much more sense to have the keyrings subsystem store the payload with better > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > David, what do you think? > > Does that even work? That should work. > Might be better to just insert 6 bytes of padding with a comment, but yes I > agree that it's probably better to align it to at least machine word size. Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should make it always right automatically. A. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption 2019-01-10 22:29 ` Aaro Koskinen @ 2019-01-10 23:01 ` Eric Biggers -1 siblings, 0 replies; 11+ messages in thread From: Eric Biggers @ 2019-01-10 23:01 UTC (permalink / raw) To: Aaro Koskinen Cc: David Howells, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings On Fri, Jan 11, 2019 at 12:29:28AM +0200, Aaro Koskinen wrote: > Hi, > > On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > > Eric Biggers <ebiggers@kernel.org> wrote: > > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > > much more sense to have the keyrings subsystem store the payload with better > > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > > > David, what do you think? > > > > Does that even work? > > That should work. > > > Might be better to just insert 6 bytes of padding with a comment, but yes I > > agree that it's probably better to align it to at least machine word size. > > Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should > make it always right automatically. > > A. I agree that __aligned is better. It should work; see 'struct crypto_tfm' in include/linux/crypto.h for another example of a struct that uses __aligned on a flexible array at the end. Aaro, can you send a formal patch? If you don't I'll do so, but I figure I'll ask first. Thanks, - Eric ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption @ 2019-01-10 23:01 ` Eric Biggers 0 siblings, 0 replies; 11+ messages in thread From: Eric Biggers @ 2019-01-10 23:01 UTC (permalink / raw) To: Aaro Koskinen Cc: David Howells, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings On Fri, Jan 11, 2019 at 12:29:28AM +0200, Aaro Koskinen wrote: > Hi, > > On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > > Eric Biggers <ebiggers@kernel.org> wrote: > > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > > much more sense to have the keyrings subsystem store the payload with better > > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > > > David, what do you think? > > > > Does that even work? > > That should work. > > > Might be better to just insert 6 bytes of padding with a comment, but yes I > > agree that it's probably better to align it to at least machine word size. > > Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should > make it always right automatically. > > A. I agree that __aligned is better. It should work; see 'struct crypto_tfm' in include/linux/crypto.h for another example of a struct that uses __aligned on a flexible array at the end. Aaro, can you send a formal patch? If you don't I'll do so, but I figure I'll ask first. Thanks, - Eric ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption 2019-01-10 23:01 ` Eric Biggers @ 2019-01-10 23:35 ` Aaro Koskinen -1 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2019-01-10 23:35 UTC (permalink / raw) To: Eric Biggers Cc: David Howells, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings Hi, On Thu, Jan 10, 2019 at 03:01:14PM -0800, Eric Biggers wrote: > On Fri, Jan 11, 2019 at 12:29:28AM +0200, Aaro Koskinen wrote: > > Hi, > > > > On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > > > Eric Biggers <ebiggers@kernel.org> wrote: > > > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > > > much more sense to have the keyrings subsystem store the payload with better > > > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > > > > > David, what do you think? > > > > > > Does that even work? > > > > That should work. > > > > > Might be better to just insert 6 bytes of padding with a comment, but yes I > > > agree that it's probably better to align it to at least machine word size. > > > > Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should > > make it always right automatically. > > > > A. > > I agree that __aligned is better. It should work; see 'struct crypto_tfm' in > include/linux/crypto.h for another example of a struct that uses __aligned on a > flexible array at the end. > > Aaro, can you send a formal patch? If you don't I'll do so, but I figure I'll > ask first. Please go ahead; I'd prefer if you send the patch, I will then test it on SPARC and reply with Tested-by (if it works :). A. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug report: unaligned access with ext4 encryption @ 2019-01-10 23:35 ` Aaro Koskinen 0 siblings, 0 replies; 11+ messages in thread From: Aaro Koskinen @ 2019-01-10 23:35 UTC (permalink / raw) To: Eric Biggers Cc: David Howells, Theodore Y. Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel, keyrings Hi, On Thu, Jan 10, 2019 at 03:01:14PM -0800, Eric Biggers wrote: > On Fri, Jan 11, 2019 at 12:29:28AM +0200, Aaro Koskinen wrote: > > Hi, > > > > On Fri, Jan 04, 2019 at 05:28:02PM +0000, David Howells wrote: > > > Eric Biggers <ebiggers@kernel.org> wrote: > > > > Hi Aaro, thanks for the bug report! I think you're on the right track; it makes > > > > much more sense to have the keyrings subsystem store the payload with better > > > > alignment, than to work around the 2-byte alignment in fscrypt. > > > > > > > > But how about '__aligned(__alignof__(u64))' instead? 4 bytes may not be enough. > > > > > > > > David, what do you think? > > > > > > Does that even work? > > > > That should work. > > > > > Might be better to just insert 6 bytes of padding with a comment, but yes I > > > agree that it's probably better to align it to at least machine word size. > > > > Padding is fragile, e.g. if struct rcu_head changes. Using __aligned should > > make it always right automatically. > > > > A. > > I agree that __aligned is better. It should work; see 'struct crypto_tfm' in > include/linux/crypto.h for another example of a struct that uses __aligned on a > flexible array at the end. > > Aaro, can you send a formal patch? If you don't I'll do so, but I figure I'll > ask first. Please go ahead; I'd prefer if you send the patch, I will then test it on SPARC and reply with Tested-by (if it works :). A. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-01-10 23:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-30 16:29 Bug report: unaligned access with ext4 encryption Aaro Koskinen 2018-12-30 16:29 ` Aaro Koskinen 2019-01-03 17:17 ` Eric Biggers 2019-01-03 17:17 ` Eric Biggers 2019-01-04 17:28 ` David Howells 2019-01-10 22:29 ` Aaro Koskinen 2019-01-10 22:29 ` Aaro Koskinen 2019-01-10 23:01 ` Eric Biggers 2019-01-10 23:01 ` Eric Biggers 2019-01-10 23:35 ` Aaro Koskinen 2019-01-10 23:35 ` Aaro Koskinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.