Re: Add fchmodat2() - or add a more general syscall?
On Tue, Jul 25, 2023 at 04:58:34PM +0100, David Howells wrote: > Rather than adding a fchmodat2() syscall, should we add a "set_file_attrs()" > syscall that takes a mask and allows you to set a bunch of stuff all in one > go? Basically, an interface to notify_change() in the kernel that would allow > several stats to be set atomically. This might be of particular interest to > network filesystems. > > David > fchmodat2() is a simple addition that fits well with the existing syscalls. It fixes an oversight in fchmodat(). IMO we should just add fchmodat2(), and not get sidetracked by trying to add some super-generalized syscall instead. That can always be done later. - Eric
Re: [RFC PATCH 01/21] crypto: scomp - Revert "add support for deflate rfc1950 (zlib)"
On Tue, Jul 18, 2023 at 03:32:39PM -0700, Eric Biggers wrote: > On Tue, Jul 18, 2023 at 02:58:27PM +0200, Ard Biesheuvel wrote: > > This reverts commit a368f43d6e3a001e684e9191a27df384fbff12f5. > > > > "zlib-deflate" was introduced 6 years ago, but it does not have any > > users. So let's remove the generic implementation and the test vectors, > > but retain the "zlib-deflate" entry in the testmgr code to avoid > > introducing warning messages on systems that implement zlib-deflate in > > hardware. > > > > Note that RFC 1950 which forms the basis of this algorithm dates back to > > 1996, and predates RFC 1951, on which the existing IPcomp is based and > > which we have supported in the kernel since 2003. So it seems rather > > unlikely that we will ever grow the need to support zlib-deflate. > > > > Signed-off-by: Ard Biesheuvel > > --- > > crypto/deflate.c | 61 +--- > > crypto/testmgr.c | 8 +-- > > crypto/testmgr.h | 75 > > 3 files changed, 18 insertions(+), 126 deletions(-) > > So if this is really unused, it's probably fair to remove it on that basis. > However, it's not correct to claim that DEFLATE is obsoleted by zlib (the data > format). zlib is just DEFLATE plus a checksum, as is gzip. > > Many users of zlib or gzip use an external checksum and therefore would be > better served by DEFLATE, avoiding a redundant builtin checksum. Typically, > people have chosen zlib or gzip simply because their compression library > defaulted to it, they didn't understand the difference, and they overlooked > that > they're paying the price for a redundant builtin checksum. > > An example of someone doing it right is EROFS, which is working on adding > DEFLATE support (not zlib or gzip!): > https://lore.kernel.org/r/20230713001441.30462-1-hsiang...@linux.alibaba.com > > Of course, they are using the library API instead of the clumsy crypto API. > Ah, I misread this patch, sorry. It's actually removing support for zlib (the data format) from the scomp API, leaving just DEFLATE. That's fine too; again, it ultimately just depends on what is actually being used via the scomp API. But similarly you can't really claim that zlib is obsoleted by DEFLATE just because of the RFC dates. As I mentioned, many people do use zlib (the data format), often just because it's the default of zlib (the library) and they didn't know any better. For example, btrfs compression supports zlib. - Eric
Re: [RFC PATCH 05/21] ubifs: Pass worst-case buffer size to compression routines
On Tue, Jul 18, 2023 at 02:58:31PM +0200, Ard Biesheuvel wrote: > Currently, the ubifs code allocates a worst case buffer size to > recompress a data node, but does not pass the size of that buffer to the > compression code. This means that the compression code will never use > the additional space, and might fail spuriously due to lack of space. > > So let's multiply out_len by WORST_COMPR_FACTOR after allocating the > buffer. Doing so is guaranteed not to overflow, given that the preceding > kmalloc_array() call would have failed otherwise. > > Signed-off-by: Ard Biesheuvel > --- > fs/ubifs/journal.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c > index dc52ac0f4a345f30..4e5961878f336033 100644 > --- a/fs/ubifs/journal.c > +++ b/fs/ubifs/journal.c > @@ -1493,6 +1493,8 @@ static int truncate_data_node(const struct ubifs_info > *c, const struct inode *in > if (!buf) > return -ENOMEM; > > + out_len *= WORST_COMPR_FACTOR; > + > dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ; > data_size = dn_size - UBIFS_DATA_NODE_SZ; > compr_type = le16_to_cpu(dn->compr_type); This looks like another case where data that would be expanded by compression should just be stored uncompressed instead. In fact, it seems that UBIFS does that already. ubifs_compress() has this: /* * If the data compressed only slightly, it is better to leave it * uncompressed to improve read speed. */ if (in_len - *out_len < UBIFS_MIN_COMPRESS_DIFF) goto no_compr; So it's unclear why the WORST_COMPR_FACTOR thing is needed at all. - Eric
Re: [RFC PATCH 01/21] crypto: scomp - Revert "add support for deflate rfc1950 (zlib)"
On Tue, Jul 18, 2023 at 02:58:27PM +0200, Ard Biesheuvel wrote: > This reverts commit a368f43d6e3a001e684e9191a27df384fbff12f5. > > "zlib-deflate" was introduced 6 years ago, but it does not have any > users. So let's remove the generic implementation and the test vectors, > but retain the "zlib-deflate" entry in the testmgr code to avoid > introducing warning messages on systems that implement zlib-deflate in > hardware. > > Note that RFC 1950 which forms the basis of this algorithm dates back to > 1996, and predates RFC 1951, on which the existing IPcomp is based and > which we have supported in the kernel since 2003. So it seems rather > unlikely that we will ever grow the need to support zlib-deflate. > > Signed-off-by: Ard Biesheuvel > --- > crypto/deflate.c | 61 +--- > crypto/testmgr.c | 8 +-- > crypto/testmgr.h | 75 > 3 files changed, 18 insertions(+), 126 deletions(-) So if this is really unused, it's probably fair to remove it on that basis. However, it's not correct to claim that DEFLATE is obsoleted by zlib (the data format). zlib is just DEFLATE plus a checksum, as is gzip. Many users of zlib or gzip use an external checksum and therefore would be better served by DEFLATE, avoiding a redundant builtin checksum. Typically, people have chosen zlib or gzip simply because their compression library defaulted to it, they didn't understand the difference, and they overlooked that they're paying the price for a redundant builtin checksum. An example of someone doing it right is EROFS, which is working on adding DEFLATE support (not zlib or gzip!): https://lore.kernel.org/r/20230713001441.30462-1-hsiang...@linux.alibaba.com Of course, they are using the library API instead of the clumsy crypto API. - Eric
Re: [PATCH v12 0/6] implement getrandom() in vDSO
On Wed, Dec 21, 2022 at 03:25:49PM +0100, Jason A. Donenfeld wrote: > On Tue, Dec 20, 2022 at 08:13:14PM +0000, Eric Biggers wrote: > > On Tue, Dec 20, 2022 at 05:17:52PM +, Christophe Leroy wrote: > > > Hi Jason, > > > > > > Le 12/12/2022 à 19:53, Jason A. Donenfeld a écrit : > > > > Changes v11->v12: > > > > > > > > - In order to avoid mlock()ing pages, and the related rlimit and fork > > > >inheritance issues there, Introduce VM_DROPPABLE to prevent swapping > > > >while meeting the cache-like requirements of vDSO getrandom(). > > > > > > > >This has some tenticles in mm/ and arch/x86/ code, so I've marked the > > > >two patches for that as still RFC, while the rest of the series is > > > > not > > > >RFC. > > > > > > > > - Mandate that opaque state blobs don't straddle page boundaries, so > > > >that VM_DROPPABLE can work on page-level granularity rather than > > > >allocation-level granularity. > > > > > > > > - Add compiler barriers to vDSO getrandom() to prevent theoretical > > > >reordering potential. > > > > > > > > - Initialize the trials loop counter in the chacha test. > > > > > > I would have liked to give it a try on powerpc, but the series > > > conflicts. I tried both on v6.1 and on linus/master from now: > > > > > > > Same here, I can't figure out how to apply this series. > > Rebased v13 posted: > https://lore.kernel.org/all/20221221142327.126451-1-ja...@zx2c4.com/ > Thanks, it is always good to give the *actual* base commit though, preferably with the --base option to git format-patch. "Latest random.git master branch" changes over time. - Eric
Re: [PATCH v12 0/6] implement getrandom() in vDSO
On Tue, Dec 20, 2022 at 05:17:52PM +, Christophe Leroy wrote: > Hi Jason, > > Le 12/12/2022 à 19:53, Jason A. Donenfeld a écrit : > > Changes v11->v12: > > > > - In order to avoid mlock()ing pages, and the related rlimit and fork > >inheritance issues there, Introduce VM_DROPPABLE to prevent swapping > >while meeting the cache-like requirements of vDSO getrandom(). > > > >This has some tenticles in mm/ and arch/x86/ code, so I've marked the > >two patches for that as still RFC, while the rest of the series is not > >RFC. > > > > - Mandate that opaque state blobs don't straddle page boundaries, so > >that VM_DROPPABLE can work on page-level granularity rather than > >allocation-level granularity. > > > > - Add compiler barriers to vDSO getrandom() to prevent theoretical > >reordering potential. > > > > - Initialize the trials loop counter in the chacha test. > > I would have liked to give it a try on powerpc, but the series > conflicts. I tried both on v6.1 and on linus/master from now: > Same here, I can't figure out how to apply this series. It would help if people always used the --base option to git format-patch... - Eric
Re: [PATCH 0/4] crypto: nintendo-aes - add a new AES driver
On Tue, Sep 21, 2021 at 11:39:26PM +0200, Emmanuel Gil Peyrot wrote: > This engine implements AES in CBC mode, using 128-bit keys only. It is > present on both the Wii and the Wii U, and is apparently identical in > both consoles. > > The hardware is capable of firing an interrupt when the operation is > done, but this driver currently uses a busy loop, I’m not too sure > whether it would be preferable to switch, nor how to achieve that. > > It also supports a mode where no operation is done, and thus could be > used as a DMA copy engine, but I don’t know how to expose that to the > kernel or whether it would even be useful. > > In my testing, on a Wii U, this driver reaches 80.7 MiB/s, while the > aes-generic driver only reaches 30.9 MiB/s, so it is a quite welcome > speedup. > > This driver was written based on reversed documentation, see: > https://wiibrew.org/wiki/Hardware/AES > > Emmanuel Gil Peyrot (4): > crypto: nintendo-aes - add a new AES driver > dt-bindings: nintendo-aes: Document the Wii and Wii U AES support > powerpc: wii.dts: Expose the AES engine on this platform > powerpc: wii_defconfig: Enable AES by default Does this pass the self-tests, including the fuzz tests which are enabled by CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y? - Eric
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 10:00:44AM -0700, Nick Desaulniers wrote:
> On Thu, Oct 22, 2020 at 9:40 AM Matthew Wilcox wrote:
> >
> > On Thu, Oct 22, 2020 at 04:35:17PM +, David Laight wrote:
> > > Wait...
> > > readv(2) defines:
> > > ssize_t readv(int fd, const struct iovec *iov, int iovcnt);
> >
> > It doesn't really matter what the manpage says. What does the AOSP
> > libc header say?
>
> Same:
> https://android.googlesource.com/platform/bionic/+/refs/heads/master/libc/include/sys/uio.h#38
>
> Theoretically someone could bypass libc to make a system call, right?
>
> >
> > > But the syscall is defined as:
> > >
> > > SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *,
> > > vec,
> > > unsigned long, vlen)
> > > {
> > > return do_readv(fd, vec, vlen, 0);
> > > }
> >
>
FWIW, glibc makes the readv() syscall assuming that fd and vlen are 'int' as
well. So this problem isn't specific to Android's libc.
>From objdump -d /lib/x86_64-linux-gnu/libc.so.6:
000f4db0 :
f4db0: 64 8b 04 25 18 00 00mov%fs:0x18,%eax
f4db7: 00
f4db8: 85 c0 test %eax,%eax
f4dba: 75 14 jnef4dd0
f4dbc: b8 13 00 00 00 mov$0x13,%eax
f4dc1: 0f 05 syscall
...
There's some code for pthread cancellation, but no zeroing of the upper half of
the fd and vlen arguments, which are in %edi and %edx respectively. But the
glibc function prototype uses 'int' for them, not 'unsigned long'
'ssize_t readv(int fd, const struct iovec *iov, int iovcnt);'.
So the high halves of the fd and iovcnt registers can contain garbage. Or at
least that's what gcc (9.3.0) and clang (9.0.1) assume; they both compile the
following
void g(unsigned int x);
void f(unsigned long x)
{
g(x);
}
into f() making a tail call to g(), without zeroing the top half of %rdi.
Also note the following program succeeds on Linux 5.9 on x86_64. On kernels
that have this bug, it should fail. (I couldn't get it to actually fail, so it
must depend on the compiler and/or the kernel config...)
#include
#include
#include
#include
#include
int main()
{
int fd = open("/dev/zero", O_RDONLY);
char buf[1000];
struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
long ret;
ret = syscall(__NR_readv, fd, , 0x10001);
if (ret < 0)
perror("readv failed");
else
printf("read %ld bytes\n", ret);
}
I think the right fix is to change the readv() (and writev(), etc.) syscalls to
take 'unsigned int' rather than 'unsigned long', as that is what the users are
assuming...
- Eric
Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
On Sun, Oct 11, 2020 at 11:56:35PM -0700, Ira Weiny wrote: > > > > And I still don't really understand. After this patchset, there is still > > code > > nearly identical to the above (doing a temporary mapping just for a memcpy) > > that > > would still be using kmap_atomic(). > > I don't understand. You mean there would be other call sites calling: > > kmap_atomic() > memcpy() > kunmap_atomic() Yes, there are tons of places that do this. Try 'git grep -A6 kmap_atomic' and look for memcpy(). Hence why I'm asking what will be the "recommended" way to do this... kunmap_thread() or kmap_atomic()? > And since I don't know the call site details if there are kmap_thread() calls > which are better off as kmap_atomic() calls I think it is worth converting > them. But I made the assumption that kmap users would already be calling > kmap_atomic() if they could (because it is more efficient). Not necessarily. In cases where either one is correct, people might not have put much thought into which of kmap() and kmap_atomic() they are using. - Eric
Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
On Sat, Oct 10, 2020 at 01:39:54AM +0100, Matthew Wilcox wrote:
> On Fri, Oct 09, 2020 at 02:34:34PM -0700, Eric Biggers wrote:
> > On Fri, Oct 09, 2020 at 12:49:57PM -0700, ira.we...@intel.com wrote:
> > > The kmap() calls in this FS are localized to a single thread. To avoid
> > > the over head of global PKRS updates use the new kmap_thread() call.
> > >
> > > @@ -2410,12 +2410,12 @@ static inline struct page
> > > *f2fs_pagecache_get_page(
> > >
> > > static inline void f2fs_copy_page(struct page *src, struct page *dst)
> > > {
> > > - char *src_kaddr = kmap(src);
> > > - char *dst_kaddr = kmap(dst);
> > > + char *src_kaddr = kmap_thread(src);
> > > + char *dst_kaddr = kmap_thread(dst);
> > >
> > > memcpy(dst_kaddr, src_kaddr, PAGE_SIZE);
> > > - kunmap(dst);
> > > - kunmap(src);
> > > + kunmap_thread(dst);
> > > + kunmap_thread(src);
> > > }
> >
> > Wouldn't it make more sense to switch cases like this to kmap_atomic()?
> > The pages are only mapped to do a memcpy(), then they're immediately
> > unmapped.
>
> Maybe you missed the earlier thread from Thomas trying to do something
> similar for rather different reasons ...
>
> https://lore.kernel.org/lkml/20200919091751.06...@linutronix.de/
I did miss it. I'm not subscribed to any of the mailing lists it was sent to.
Anyway, it shouldn't matter. Patchsets should be standalone, and not require
reading random prior threads on linux-kernel to understand.
And I still don't really understand. After this patchset, there is still code
nearly identical to the above (doing a temporary mapping just for a memcpy) that
would still be using kmap_atomic(). Is the idea that later, such code will be
converted to use kmap_thread() instead? If not, why use one over the other?
- Eric
Re: [PATCH 05/14] fs: don't allow kernel reads and writes without iter ops
On Fri, Oct 09, 2020 at 06:03:31PM -0700, Linus Torvalds wrote: > On Fri, Oct 9, 2020 at 3:06 PM Eric Biggers wrote: > > > > It's a bit unintuitive that ppos=NULL means "use pos 0", not "use > > file->f_pos". > > That's not at all what it means. > > A NULL ppos means "this has no position at all", and is what we use > for FMODE_STREAM file descriptors (ie sockets, pipes, etc). > > It also means that we don't do the locking for position updates. > > The fact that "ki_pos" gets set to zero is just because it needs to be > _something_. It shouldn't actually ever be used for stream devices. > Okay, that makes more sense. So the patchset from Matthew https://lkml.kernel.org/linux-fsdevel/20201003025534.21045-1-wi...@infradead.org/T/#u isn't what you had in mind. - Eric
Re: [PATCH 05/14] fs: don't allow kernel reads and writes without iter ops
On Fri, Oct 02, 2020 at 09:27:09AM -0700, Linus Torvalds wrote: > On Thu, Oct 1, 2020 at 3:41 PM Al Viro wrote: > > > > Better > > loff_t dummy = 0; > > ... > > wr = __kernel_write(file, data, bytes, ); > > No, just fix __kernel_write() to work correctly. > > The fact is, NULL _is_ the right pointer for ppos these days. > > That commit by Christoph is buggy: it replaces new_sync_write() with a > buggy open-coded version. > > Notice how new_sync_write does > > kiocb.ki_pos = (ppos ? *ppos : 0); > ,,, > if (ret > 0 && ppos) > *ppos = kiocb.ki_pos; > > but the open-coded version doesn't. > > So just fix that in linux-next. The *last* thing we want is to have > different semantics for the "same" kernel functions. It's a bit unintuitive that ppos=NULL means "use pos 0", not "use file->f_pos". Anyway, it works. The important thing is, this is still broken in linux-next... - Eric
Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
On Fri, Oct 09, 2020 at 12:49:57PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny
>
> The kmap() calls in this FS are localized to a single thread. To avoid
> the over head of global PKRS updates use the new kmap_thread() call.
>
> Cc: Jaegeuk Kim
> Cc: Chao Yu
> Signed-off-by: Ira Weiny
> ---
> fs/f2fs/f2fs.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d9e52a7f3702..ff72a45a577e 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2410,12 +2410,12 @@ static inline struct page *f2fs_pagecache_get_page(
>
> static inline void f2fs_copy_page(struct page *src, struct page *dst)
> {
> - char *src_kaddr = kmap(src);
> - char *dst_kaddr = kmap(dst);
> + char *src_kaddr = kmap_thread(src);
> + char *dst_kaddr = kmap_thread(dst);
>
> memcpy(dst_kaddr, src_kaddr, PAGE_SIZE);
> - kunmap(dst);
> - kunmap(src);
> + kunmap_thread(dst);
> + kunmap_thread(src);
> }
Wouldn't it make more sense to switch cases like this to kmap_atomic()?
The pages are only mapped to do a memcpy(), then they're immediately unmapped.
- Eric
Re: [PATCH 05/14] fs: don't allow kernel reads and writes without iter ops
Christoph, Al, and Linus:
On Thu, Sep 03, 2020 at 04:22:33PM +0200, Christoph Hellwig wrote:
> @@ -510,28 +524,31 @@ static ssize_t new_sync_write(struct file *filp, const
> char __user *buf, size_t
> /* caller is responsible for file_start_write/file_end_write */
> ssize_t __kernel_write(struct file *file, const void *buf, size_t count,
> loff_t *pos)
> {
> - mm_segment_t old_fs;
> - const char __user *p;
> + struct kvec iov = {
> + .iov_base = (void *)buf,
> + .iov_len= min_t(size_t, count, MAX_RW_COUNT),
> + };
> + struct kiocb kiocb;
> + struct iov_iter iter;
> ssize_t ret;
>
> if (WARN_ON_ONCE(!(file->f_mode & FMODE_WRITE)))
> return -EBADF;
> if (!(file->f_mode & FMODE_CAN_WRITE))
> return -EINVAL;
> + /*
> + * Also fail if ->write_iter and ->write are both wired up as that
> + * implies very convoluted semantics.
> + */
> + if (unlikely(!file->f_op->write_iter || file->f_op->write))
> + return warn_unsupported(file, "write");
>
> - old_fs = get_fs();
> - set_fs(KERNEL_DS);
> - p = (__force const char __user *)buf;
> - if (count > MAX_RW_COUNT)
> - count = MAX_RW_COUNT;
> - if (file->f_op->write)
> - ret = file->f_op->write(file, p, count, pos);
> - else if (file->f_op->write_iter)
> - ret = new_sync_write(file, p, count, pos);
> - else
> - ret = -EINVAL;
> - set_fs(old_fs);
> + init_sync_kiocb(, file);
> + kiocb.ki_pos = *pos;
> + iov_iter_kvec(, WRITE, , 1, iov.iov_len);
> + ret = file->f_op->write_iter(, );
next-20201001 crashes on boot for me because there is a bad interaction between
this commit in vfs/for-next:
commit 4d03e3cc59828c82ee89ea6e27a2f3cdf95aaadf
Author: Christoph Hellwig
Date: Thu Sep 3 16:22:33 2020 +0200
fs: don't allow kernel reads and writes without iter ops
... and Linus's mainline commit:
commit 90fb702791bf99b959006972e8ee7bb4609f441b
Author: Linus Torvalds
Date: Tue Sep 29 17:18:34 2020 -0700
autofs: use __kernel_write() for the autofs pipe writing
Linus's commit made autofs start passing pos=NULL to __kernel_write().
But, Christoph's commit made __kernel_write() no longer accept a NULL pos.
The following fixes it:
diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
index 5ced859dac53..b04c528b19d3 100644
--- a/fs/autofs/waitq.c
+++ b/fs/autofs/waitq.c
@@ -53,7 +53,7 @@ static int autofs_write(struct autofs_sb_info *sbi,
mutex_lock(>pipe_mutex);
while (bytes) {
- wr = __kernel_write(file, data, bytes, NULL);
+ wr = __kernel_write(file, data, bytes, >f_pos);
if (wr <= 0)
break;
data += wr;
I'm not sure what was intended, though.
Full stack trace below.
BUG: kernel NULL pointer dereference, address:
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0 P4D 0
Oops: [#1] PREEMPT SMP NOPTI
CPU: 12 PID: 383 Comm: systemd-binfmt Tainted: GT
5.9.0-rc7-next-20201001 #2
Hardware name: Gigabyte Technology Co., Ltd. X399 AORUS Gaming 7/X399 AORUS
Gaming 7, BIOS F2 08/31/2017
RIP: 0010:init_sync_kiocb include/linux/fs.h:2050 [inline]
RIP: 0010:__kernel_write+0x16c/0x2b0 fs/read_write.c:546
Code: 24 4a b9 01 00 00 00 0f 45 c7 be 01 00 00 00 48 8d 7c 24 10 48 c7 44 24
40 00 00 00 00 48 c7 44 24 58 00 00 00 00 89 44 24 4c <48> 8b 03 48 c7 44 24 60
00 00 00 00 48 89 44 24 50 4c 89 64 24 38
RSP: 0018:a2fc0102f8b0 EFLAGS: 00010246
RAX: 0002 RBX: RCX: 0001
RDX: a2fc0102f8b0 RSI: 0001 RDI: a2fc0102f8c0
RBP: 8ad2927e2940 R08: 0130 R09: 8ad29a201800
R10: 000f R11: 8ad292547510 R12: 8ad2927e2940
R13: a2fc0102f8e8 R14: 8ad29a951768 R15: 0130
FS: 7f11023b9000() GS:8ad29ed0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: CR3: 000857b62000 CR4: 003506e0
Call Trace:
autofs_write fs/autofs/waitq.c:56 [inline]
autofs_notify_daemon.constprop.0+0x115/0x260 fs/autofs/waitq.c:163
autofs_wait+0x5b1/0x750 fs/autofs/waitq.c:465
autofs_mount_wait+0x2d/0x60 fs/autofs/root.c:250
autofs_d_automount+0xc4/0x1a0 fs/autofs/root.c:379
follow_automount fs/namei.c:1198 [inline]
__traverse_mounts+0x8d/0x230 fs/namei.c:1243
traverse_mounts fs/namei.c:1272 [inline]
handle_mounts fs/namei.c:1381 [inline]
step_into+0x44e/0x730 fs/namei.c:1691
walk_component+0x7e/0x1b0 fs/namei.c:1867
link_path_walk+0x270/0x3b0 fs/namei.c:2184
path_openat+0x90/0xe40 fs/namei.c:3365
do_filp_open+0x98/0x140 fs/namei.c:3396
do_sys_openat2+0xac/0x170 fs/open.c:1168
do_sys_open fs/open.c:1184 [inline]
__do_sys_openat
Re: [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
On Mon, Aug 10, 2020 at 05:33:39PM +0300, Horia Geantă wrote: > On 8/10/2020 4:45 PM, Herbert Xu wrote: > > On Mon, Aug 10, 2020 at 10:20:20AM +, Van Leeuwen, Pascal wrote: > >> > >> With all due respect, but this makes no sense. > > > > I agree. This is a lot of churn for no gain. > > > I would say the gain is that all skcipher algorithms would behave the same > when input length equals zero - i.e. treat the request as a no-op. > > We can't say "no input" has any meaning to the other skcipher algorithms, > but the convention is to accept this case and just return 0. > I don't see why XTS has to be handled differently. > CTS also rejects empty inputs. The rule it follows is just that all input lengths >= blocksize are allowed. Input lengths < blocksize aren't allowed. - Eric
Re: [PATCH v4 1/3] mm/slab: Use memzero_explicit() in kzfree()
On Mon, Jun 15, 2020 at 09:57:16PM -0400, Waiman Long wrote:
> The kzfree() function is normally used to clear some sensitive
> information, like encryption keys, in the buffer before freeing it back
> to the pool. Memset() is currently used for the buffer clearing. However,
> it is entirely possible that the compiler may choose to optimize away the
> memory clearing especially if LTO is being used. To make sure that this
> optimization will not happen, memzero_explicit(), which is introduced
> in v3.18, is now used in kzfree() to do the clearing.
>
> Fixes: 3ef0e5ba4673 ("slab: introduce kzfree()")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Waiman Long
> ---
> mm/slab_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 9e72ba224175..37d48a56431d 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1726,7 +1726,7 @@ void kzfree(const void *p)
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> ks = ksize(mem);
> - memset(mem, 0, ks);
> + memzero_explicit(mem, ks);
> kfree(mem);
> }
> EXPORT_SYMBOL(kzfree);
This is a good change, but the commit message isn't really accurate. AFAIK, no
one has found any case where this memset() gets optimized out. And even with
LTO, it would be virtually impossible due to all the synchronization and global
data structures that kfree() uses. (Remember that this isn't the C standard
function "free()", so the compiler can't assign it any special meaning.)
Not to mention that LTO support isn't actually upstream yet.
I still agree with the change, but it might be helpful if the commit message
were honest that this is really a hardening measure and about properly conveying
the intent. As-is this sounds like a critical fix, which might confuse people.
- Eric
Re: [PATCH 0/7] sha1 library cleanup
On Sat, May 02, 2020 at 03:05:46PM -0600, Jason A. Donenfeld wrote:
> Thanks for this series. I like the general idea. I think it might make
> sense, though, to separate things out into sha1.h and sha256.h. That
> will be nice preparation work for when we eventually move obsolete
> primitives into some subdirectory.
That's basically what I suggested in the cover letter:
"As future work, we should split sha.h into sha1.h and sha2.h and try to
remove the remaining uses of SHA-1. For example, the remaining use in
drivers/char/random.c is probably one that can be gotten rid of."
("sha2.h" rather than "sha256.h", since it would include SHA-512 too.
Also, we already have sha3.h, so having sha{1,2,3}.h would be logical.)
But there are 108 files that include , all of which would need to
be updated, which risks merge conflicts. So this series seemed like a good
stopping point to get these initial changes in for 5.8. Then in the next
release we can split up sha.h (and debate whether sha1.h should really be
"" or whatever).
There are 3 files where I added an include of sha.h, where we could go directly
to sha1.h if we did it now. But that's not much compared to the 108 files.
- Eric
[PATCH 0/7] sha1 library cleanup
sounds very generic and important, like it's the header to include if you're doing cryptographic hashing in the kernel. But actually it only includes the library implementation of the SHA-1 compression function (not even the full SHA-1). This should basically never be used anymore; SHA-1 is no longer considered secure, and there are much better ways to do cryptographic hashing in the kernel. Also the function is named just "sha_transform()", which makes it unclear which version of SHA is meant. Therefore, this series cleans things up by moving these SHA-1 declarations into where they better belong, and changing the names to say SHA-1 rather than just SHA. As future work, we should split sha.h into sha1.h and sha2.h and try to remove the remaining uses of SHA-1. For example, the remaining use in drivers/char/random.c is probably one that can be gotten rid of. This patch series applies to cryptodev/master. Eric Biggers (7): mptcp: use SHA256_BLOCK_SIZE, not SHA_MESSAGE_BYTES crypto: powerpc/sha1 - remove unused temporary workspace crypto: powerpc/sha1 - prefix the "sha1_" functions crypto: s390/sha1 - prefix the "sha1_" functions crypto: lib/sha1 - rename "sha" to "sha1" crypto: lib/sha1 - remove unnecessary includes of linux/cryptohash.h crypto: lib/sha1 - fold linux/cryptohash.h into crypto/sha.h Documentation/security/siphash.rst | 2 +- arch/arm/crypto/sha1_glue.c | 1 - arch/arm/crypto/sha1_neon_glue.c| 1 - arch/arm/crypto/sha256_glue.c | 1 - arch/arm/crypto/sha256_neon_glue.c | 1 - arch/arm/kernel/armksyms.c | 1 - arch/arm64/crypto/sha256-glue.c | 1 - arch/arm64/crypto/sha512-glue.c | 1 - arch/microblaze/kernel/microblaze_ksyms.c | 1 - arch/mips/cavium-octeon/crypto/octeon-md5.c | 1 - arch/powerpc/crypto/md5-glue.c | 1 - arch/powerpc/crypto/sha1-spe-glue.c | 1 - arch/powerpc/crypto/sha1.c | 33 ++--- arch/powerpc/crypto/sha256-spe-glue.c | 1 - arch/s390/crypto/sha1_s390.c| 12 arch/sparc/crypto/md5_glue.c| 1 - arch/sparc/crypto/sha1_glue.c | 1 - arch/sparc/crypto/sha256_glue.c | 1 - arch/sparc/crypto/sha512_glue.c | 1 - arch/unicore32/kernel/ksyms.c | 1 - arch/x86/crypto/sha1_ssse3_glue.c | 1 - arch/x86/crypto/sha256_ssse3_glue.c | 1 - arch/x86/crypto/sha512_ssse3_glue.c | 1 - crypto/sha1_generic.c | 5 ++-- drivers/char/random.c | 8 ++--- drivers/crypto/atmel-sha.c | 1 - drivers/crypto/chelsio/chcr_algo.c | 1 - drivers/crypto/chelsio/chcr_ipsec.c | 1 - drivers/crypto/omap-sham.c | 1 - fs/f2fs/hash.c | 1 - include/crypto/sha.h| 10 +++ include/linux/cryptohash.h | 14 - include/linux/filter.h | 4 +-- include/net/tcp.h | 1 - kernel/bpf/core.c | 18 +-- lib/crypto/chacha.c | 1 - lib/sha1.c | 24 --- net/core/secure_seq.c | 1 - net/ipv6/addrconf.c | 10 +++ net/ipv6/seg6_hmac.c| 1 - net/mptcp/crypto.c | 4 +-- 41 files changed, 69 insertions(+), 104 deletions(-) delete mode 100644 include/linux/cryptohash.h base-commit: 12b3cf9093542d9f752a4968815ece836159013f -- 2.26.2
[PATCH 2/7] crypto: powerpc/sha1 - remove unused temporary workspace
From: Eric Biggers
The PowerPC implementation of SHA-1 doesn't actually use the 16-word
temporary array that's passed to the assembly code. This was probably
meant to correspond to the 'W' array that lib/sha1.c uses. However, in
sha1-powerpc-asm.S these values are actually stored in GPRs 16-31.
Referencing SHA_WORKSPACE_WORDS from this code also isn't appropriate,
since it's an implementation detail of lib/sha1.c.
Therefore, just remove this unneeded array.
Tested with:
export ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
make mpc85xx_defconfig
cat >> .config << EOF
# CONFIG_MODULES is not set
# CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
CONFIG_DEBUG_KERNEL=y
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
CONFIG_CRYPTO_SHA1_PPC=y
EOF
make olddefconfig
make -j32
qemu-system-ppc -M mpc8544ds -cpu e500 -nographic \
-kernel arch/powerpc/boot/zImage \
-append "cryptomgr.fuzz_iterations=1000
cryptomgr.panic_on_fail=1"
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Michael Ellerman
Cc: Paul Mackerras
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/sha1.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 7b43fc352089b1..db46b6130a9642 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -16,12 +16,11 @@
#include
#include
#include
-#include
#include
#include
#include
-extern void powerpc_sha_transform(u32 *state, const u8 *src, u32 *temp);
+void powerpc_sha_transform(u32 *state, const u8 *src);
static int sha1_init(struct shash_desc *desc)
{
@@ -47,7 +46,6 @@ static int sha1_update(struct shash_desc *desc, const u8
*data,
src = data;
if ((partial + len) > 63) {
- u32 temp[SHA_WORKSPACE_WORDS];
if (partial) {
done = -partial;
@@ -56,12 +54,11 @@ static int sha1_update(struct shash_desc *desc, const u8
*data,
}
do {
- powerpc_sha_transform(sctx->state, src, temp);
+ powerpc_sha_transform(sctx->state, src);
done += 64;
src = data + done;
} while (done + 63 < len);
- memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx->buffer + partial, src, len - done);
--
2.26.2
[PATCH 3/7] crypto: powerpc/sha1 - prefix the "sha1_" functions
From: Eric Biggers
Prefix the PowerPC SHA-1 functions with "powerpc_sha1_" rather than
"sha1_". This allows us to rename the library function sha_init() to
sha1_init() without causing a naming collision.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Michael Ellerman
Cc: Paul Mackerras
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/sha1.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index db46b6130a9642..b40dc50a6908ae 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -22,7 +22,7 @@
void powerpc_sha_transform(u32 *state, const u8 *src);
-static int sha1_init(struct shash_desc *desc)
+static int powerpc_sha1_init(struct shash_desc *desc)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
@@ -33,8 +33,8 @@ static int sha1_init(struct shash_desc *desc)
return 0;
}
-static int sha1_update(struct shash_desc *desc, const u8 *data,
- unsigned int len)
+static int powerpc_sha1_update(struct shash_desc *desc, const u8 *data,
+ unsigned int len)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
unsigned int partial, done;
@@ -68,7 +68,7 @@ static int sha1_update(struct shash_desc *desc, const u8
*data,
/* Add padding and return the message digest. */
-static int sha1_final(struct shash_desc *desc, u8 *out)
+static int powerpc_sha1_final(struct shash_desc *desc, u8 *out)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
__be32 *dst = (__be32 *)out;
@@ -81,10 +81,10 @@ static int sha1_final(struct shash_desc *desc, u8 *out)
/* Pad out to 56 mod 64 */
index = sctx->count & 0x3f;
padlen = (index < 56) ? (56 - index) : ((64+56) - index);
- sha1_update(desc, padding, padlen);
+ powerpc_sha1_update(desc, padding, padlen);
/* Append length */
- sha1_update(desc, (const u8 *), sizeof(bits));
+ powerpc_sha1_update(desc, (const u8 *), sizeof(bits));
/* Store state in digest */
for (i = 0; i < 5; i++)
@@ -96,7 +96,7 @@ static int sha1_final(struct shash_desc *desc, u8 *out)
return 0;
}
-static int sha1_export(struct shash_desc *desc, void *out)
+static int powerpc_sha1_export(struct shash_desc *desc, void *out)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
@@ -104,7 +104,7 @@ static int sha1_export(struct shash_desc *desc, void *out)
return 0;
}
-static int sha1_import(struct shash_desc *desc, const void *in)
+static int powerpc_sha1_import(struct shash_desc *desc, const void *in)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
@@ -114,11 +114,11 @@ static int sha1_import(struct shash_desc *desc, const
void *in)
static struct shash_alg alg = {
.digestsize = SHA1_DIGEST_SIZE,
- .init = sha1_init,
- .update = sha1_update,
- .final = sha1_final,
- .export = sha1_export,
- .import = sha1_import,
+ .init = powerpc_sha1_init,
+ .update = powerpc_sha1_update,
+ .final = powerpc_sha1_final,
+ .export = powerpc_sha1_export,
+ .import = powerpc_sha1_import,
.descsize = sizeof(struct sha1_state),
.statesize = sizeof(struct sha1_state),
.base = {
--
2.26.2
[PATCH v2 0/3] crypto: powerpc - convert SPE AES algorithms to skcipher API
This series converts the glue code for the PowerPC SPE implementations of AES-ECB, AES-CBC, AES-CTR, and AES-XTS from the deprecated "blkcipher" API to the "skcipher" API. This is needed in order for the blkcipher API to be removed. Patch 1-2 are fixes. Patch 3 is the actual conversion. Tested with: export ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- make mpc85xx_defconfig cat >> .config << EOF # CONFIG_MODULES is not set # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set CONFIG_DEBUG_KERNEL=y CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y CONFIG_CRYPTO_AES=y CONFIG_CRYPTO_CBC=y CONFIG_CRYPTO_CTR=y CONFIG_CRYPTO_ECB=y CONFIG_CRYPTO_XTS=y CONFIG_CRYPTO_AES_PPC_SPE=y EOF make olddefconfig make -j32 qemu-system-ppc -M mpc8544ds -cpu e500 -nographic \ -kernel arch/powerpc/boot/zImage \ -append cryptomgr.fuzz_iterations=1000 Note that xts-ppc-spe still fails the comparison tests due to the lack of ciphertext stealing support. This is not addressed by this series. Changed since v1: - Split fixes into separate patches. - Made ppc_aes_setkey_skcipher() call ppc_aes_setkey(), rather than creating a separate expand_key() function. This keeps the code shorter. Eric Biggers (3): crypto: powerpc - don't unnecessarily use atomic scatterwalk crypto: powerpc - don't set ivsize for AES-ECB crypto: powerpc - convert SPE AES algorithms to skcipher API arch/powerpc/crypto/aes-spe-glue.c | 389 - crypto/Kconfig | 1 + 2 files changed, 166 insertions(+), 224 deletions(-) -- 2.23.0
[PATCH v2 2/3] crypto: powerpc - don't set ivsize for AES-ECB
From: Eric Biggers
Set the ivsize for the "ecb-ppc-spe" algorithm to 0, since ECB mode
doesn't take an IV.
This fixes a failure in the extra crypto self-tests:
alg: skcipher: ivsize for ecb-ppc-spe (16) doesn't match generic impl
(0)
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/aes-spe-glue.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/crypto/aes-spe-glue.c
b/arch/powerpc/crypto/aes-spe-glue.c
index 319f1dbb3a70..4189d2644f74 100644
--- a/arch/powerpc/crypto/aes-spe-glue.c
+++ b/arch/powerpc/crypto/aes-spe-glue.c
@@ -415,7 +415,6 @@ static struct crypto_alg aes_algs[] = { {
.blkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
.setkey = ppc_aes_setkey,
.encrypt= ppc_ecb_encrypt,
.decrypt= ppc_ecb_decrypt,
--
2.23.0
[PATCH v2 3/3] crypto: powerpc - convert SPE AES algorithms to skcipher API
From: Eric Biggers
Convert the glue code for the PowerPC SPE implementations of AES-ECB,
AES-CBC, AES-CTR, and AES-XTS from the deprecated "blkcipher" API to the
"skcipher" API. This is needed in order for the blkcipher API to be
removed.
Tested with:
export ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
make mpc85xx_defconfig
cat >> .config << EOF
# CONFIG_MODULES is not set
# CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
CONFIG_DEBUG_KERNEL=y
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
CONFIG_CRYPTO_AES=y
CONFIG_CRYPTO_CBC=y
CONFIG_CRYPTO_CTR=y
CONFIG_CRYPTO_ECB=y
CONFIG_CRYPTO_XTS=y
CONFIG_CRYPTO_AES_PPC_SPE=y
EOF
make olddefconfig
make -j32
qemu-system-ppc -M mpc8544ds -cpu e500 -nographic \
-kernel arch/powerpc/boot/zImage \
-append cryptomgr.fuzz_iterations=1000
Note that xts-ppc-spe still fails the comparison tests due to the lack
of ciphertext stealing support. This is not addressed by this patch.
This patch also cleans up the code by making ->encrypt() and ->decrypt()
call a common function for each of ECB, CBC, and XTS, and by using a
clearer way to compute the length to process at each step.
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/aes-spe-glue.c | 381 +
crypto/Kconfig | 1 +
2 files changed, 166 insertions(+), 216 deletions(-)
diff --git a/arch/powerpc/crypto/aes-spe-glue.c
b/arch/powerpc/crypto/aes-spe-glue.c
index 4189d2644f74..f828f8bcd0c6 100644
--- a/arch/powerpc/crypto/aes-spe-glue.c
+++ b/arch/powerpc/crypto/aes-spe-glue.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
/*
@@ -118,13 +119,19 @@ static int ppc_aes_setkey(struct crypto_tfm *tfm, const
u8 *in_key,
return 0;
}
-static int ppc_xts_setkey(struct crypto_tfm *tfm, const u8 *in_key,
+static int ppc_aes_setkey_skcipher(struct crypto_skcipher *tfm,
+ const u8 *in_key, unsigned int key_len)
+{
+ return ppc_aes_setkey(crypto_skcipher_tfm(tfm), in_key, key_len);
+}
+
+static int ppc_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
unsigned int key_len)
{
- struct ppc_xts_ctx *ctx = crypto_tfm_ctx(tfm);
+ struct ppc_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
int err;
- err = xts_check_key(tfm, in_key, key_len);
+ err = xts_verify_key(tfm, in_key, key_len);
if (err)
return err;
@@ -133,7 +140,7 @@ static int ppc_xts_setkey(struct crypto_tfm *tfm, const u8
*in_key,
if (key_len != AES_KEYSIZE_128 &&
key_len != AES_KEYSIZE_192 &&
key_len != AES_KEYSIZE_256) {
- tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+ crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
return -EINVAL;
}
@@ -178,201 +185,154 @@ static void ppc_aes_decrypt(struct crypto_tfm *tfm, u8
*out, const u8 *in)
spe_end();
}
-static int ppc_ecb_encrypt(struct blkcipher_desc *desc, struct scatterlist
*dst,
- struct scatterlist *src, unsigned int nbytes)
+static int ppc_ecb_crypt(struct skcipher_request *req, bool enc)
{
- struct ppc_aes_ctx *ctx = crypto_blkcipher_ctx(desc->tfm);
- struct blkcipher_walk walk;
- unsigned int ubytes;
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct ppc_aes_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct skcipher_walk walk;
+ unsigned int nbytes;
int err;
- blkcipher_walk_init(, dst, src, nbytes);
- err = blkcipher_walk_virt(desc, );
+ err = skcipher_walk_virt(, req, false);
- while ((nbytes = walk.nbytes)) {
- ubytes = nbytes > MAX_BYTES ?
-nbytes - MAX_BYTES : nbytes & (AES_BLOCK_SIZE - 1);
- nbytes -= ubytes;
+ while ((nbytes = walk.nbytes) != 0) {
+ nbytes = min_t(unsigned int, nbytes, MAX_BYTES);
+ nbytes = round_down(nbytes, AES_BLOCK_SIZE);
spe_begin();
- ppc_encrypt_ecb(walk.dst.virt.addr, walk.src.virt.addr,
- ctx->key_enc, ctx->rounds, nbytes);
+ if (enc)
+ ppc_encrypt_ecb(walk.dst.virt.addr, walk.src.virt.addr,
+ ctx->key_enc, ctx->rounds, nbytes);
+ else
+ ppc_decrypt_ecb(walk.dst.virt.addr, walk.src.virt.addr,
+ ctx->key_dec, ctx->rounds, nbytes);
spe_end();
- err = blkcipher_walk_done(desc, , ubytes);
+ err = skcipher_walk_done(, walk.nbytes - nbytes);
}
return err;
[PATCH v2 1/3] crypto: powerpc - don't unnecessarily use atomic scatterwalk
From: Eric Biggers The PowerPC SPE implementations of AES modes only disable preemption during the actual encryption/decryption, not during the scatterwalk functions. It's therefore unnecessary to request an atomic scatterwalk. So don't do so. Signed-off-by: Eric Biggers --- arch/powerpc/crypto/aes-spe-glue.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/arch/powerpc/crypto/aes-spe-glue.c b/arch/powerpc/crypto/aes-spe-glue.c index 3a4ca7d32477..319f1dbb3a70 100644 --- a/arch/powerpc/crypto/aes-spe-glue.c +++ b/arch/powerpc/crypto/aes-spe-glue.c @@ -186,7 +186,6 @@ static int ppc_ecb_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst, unsigned int ubytes; int err; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); @@ -214,7 +213,6 @@ static int ppc_ecb_decrypt(struct blkcipher_desc *desc, struct scatterlist *dst, unsigned int ubytes; int err; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); @@ -242,7 +240,6 @@ static int ppc_cbc_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst, unsigned int ubytes; int err; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); @@ -270,7 +267,6 @@ static int ppc_cbc_decrypt(struct blkcipher_desc *desc, struct scatterlist *dst, unsigned int ubytes; int err; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); @@ -298,7 +294,6 @@ static int ppc_ctr_crypt(struct blkcipher_desc *desc, struct scatterlist *dst, unsigned int pbytes, ubytes; int err; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt_block(desc, , AES_BLOCK_SIZE); @@ -329,7 +324,6 @@ static int ppc_xts_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst, int err; u32 *twk; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); twk = ctx->key_twk; @@ -360,7 +354,6 @@ static int ppc_xts_decrypt(struct blkcipher_desc *desc, struct scatterlist *dst, int err; u32 *twk; - desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP; blkcipher_walk_init(, dst, src, nbytes); err = blkcipher_walk_virt(desc, ); twk = ctx->key_twk; -- 2.23.0
Re: [PATCH] crypto: powerpc - convert SPE AES algorithms to skcipher API
On Mon, Oct 14, 2019 at 10:45:22AM +0200, Ard Biesheuvel wrote:
> Hi Eric,
>
> On Sat, 12 Oct 2019 at 04:32, Eric Biggers wrote:
> >
> > From: Eric Biggers
> >
> > Convert the glue code for the PowerPC SPE implementations of AES-ECB,
> > AES-CBC, AES-CTR, and AES-XTS from the deprecated "blkcipher" API to the
> > "skcipher" API.
> >
> > Tested with:
> >
> > export ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
> > make mpc85xx_defconfig
> > cat >> .config << EOF
> > # CONFIG_MODULES is not set
> > # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
> > CONFIG_DEBUG_KERNEL=y
> > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
> > CONFIG_CRYPTO_AES=y
> > CONFIG_CRYPTO_CBC=y
> > CONFIG_CRYPTO_CTR=y
> > CONFIG_CRYPTO_ECB=y
> > CONFIG_CRYPTO_XTS=y
> > CONFIG_CRYPTO_AES_PPC_SPE=y
> > EOF
> > make olddefconfig
> > make -j32
> > qemu-system-ppc -M mpc8544ds -cpu e500 -nographic \
> > -kernel arch/powerpc/boot/zImage \
> > -append cryptomgr.fuzz_iterations=1000
> >
> > Note that xts-ppc-spe still fails the comparison tests due to the lack
> > of ciphertext stealing support. This is not addressed by this patch.
> >
> > Signed-off-by: Eric Biggers
> > ---
> > arch/powerpc/crypto/aes-spe-glue.c | 416 +
> > crypto/Kconfig | 1 +
> > 2 files changed, 186 insertions(+), 231 deletions(-)
> >
> > diff --git a/arch/powerpc/crypto/aes-spe-glue.c
> > b/arch/powerpc/crypto/aes-spe-glue.c
> > index 3a4ca7d32477..374e3e51e998 100644
> > --- a/arch/powerpc/crypto/aes-spe-glue.c
> > +++ b/arch/powerpc/crypto/aes-spe-glue.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > /*
> > @@ -86,17 +87,13 @@ static void spe_end(void)
> > preempt_enable();
> > }
> >
> > -static int ppc_aes_setkey(struct crypto_tfm *tfm, const u8 *in_key,
> > - unsigned int key_len)
> > +static int expand_key(struct ppc_aes_ctx *ctx,
> > + const u8 *in_key, unsigned int key_len)
> > {
> > - struct ppc_aes_ctx *ctx = crypto_tfm_ctx(tfm);
> > -
> > if (key_len != AES_KEYSIZE_128 &&
> > key_len != AES_KEYSIZE_192 &&
> > - key_len != AES_KEYSIZE_256) {
> > - tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
> > + key_len != AES_KEYSIZE_256)
> > return -EINVAL;
> > - }
> >
> > switch (key_len) {
> > case AES_KEYSIZE_128:
> > @@ -114,17 +111,40 @@ static int ppc_aes_setkey(struct crypto_tfm *tfm,
> > const u8 *in_key,
> > }
> >
> > ppc_generate_decrypt_key(ctx->key_dec, ctx->key_enc, key_len);
> > + return 0;
> > +}
> >
> > +static int ppc_aes_setkey(struct crypto_tfm *tfm, const u8 *in_key,
> > + unsigned int key_len)
> > +{
> > + struct ppc_aes_ctx *ctx = crypto_tfm_ctx(tfm);
> > +
> > + if (expand_key(ctx, in_key, key_len) != 0) {
> > + tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int ppc_aes_setkey_skcipher(struct crypto_skcipher *tfm,
> > + const u8 *in_key, unsigned int key_len)
> > +{
> > + struct ppc_aes_ctx *ctx = crypto_skcipher_ctx(tfm);
> > +
> > + if (expand_key(ctx, in_key, key_len) != 0) {
> > + crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> > + return -EINVAL;
> > + }
> > return 0;
> > }
> >
> > -static int ppc_xts_setkey(struct crypto_tfm *tfm, const u8 *in_key,
> > +static int ppc_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
> >unsigned int key_len)
> > {
> > - struct ppc_xts_ctx *ctx = crypto_tfm_ctx(tfm);
> > + struct ppc_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
> > int err;
> >
> > - err = xts_check_key(tfm, in_key, key_len);
> > + err = xts_verify_key(tfm, in_key, key_len);
> > if (err)
> > return err;
> >
> > @@ -133,7 +153,7 @@ st
Re: [PATCH 0/4] crypto: nx - convert to skcipher API
On Sun, Oct 13, 2019 at 05:31:31PM +0200, Ard Biesheuvel wrote: > On Sun, 13 Oct 2019 at 08:29, Ard Biesheuvel > wrote: > > > > On Sun, 13 Oct 2019 at 06:40, Eric Biggers wrote: > > > > > > This series converts the PowerPC Nest (NX) implementations of AES modes > > > from the deprecated "blkcipher" API to the "skcipher" API. This is > > > needed in order for the blkcipher API to be removed. > > > > > > This patchset is compile-tested only, as I don't have this hardware. > > > If anyone has this hardware, please test this patchset with > > > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y. > > > > > > Eric Biggers (4): > > > crypto: nx - don't abuse blkcipher_desc to pass iv around > > > crypto: nx - convert AES-ECB to skcipher API > > > crypto: nx - convert AES-CBC to skcipher API > > > crypto: nx - convert AES-CTR to skcipher API > > > > > > drivers/crypto/nx/nx-aes-cbc.c | 81 ++- > > > drivers/crypto/nx/nx-aes-ccm.c | 40 ++-- > > > drivers/crypto/nx/nx-aes-ctr.c | 87 +++--- > > > drivers/crypto/nx/nx-aes-ecb.c | 76 + > > > drivers/crypto/nx/nx-aes-gcm.c | 24 -- > > > drivers/crypto/nx/nx.c | 64 ++--- > > > drivers/crypto/nx/nx.h | 19 > > > 7 files changed, 176 insertions(+), 215 deletions(-) > > > > > > > Hi Eric, > > > > Thanks for taking this on. I'll look in more detail at these patches > > during the week. In the meantime, I may have a stab at converting ccp, > > virtio-crypto and omap aes/des myself, since i have the hardware to > > test those. > > > > OK, I got a bit carried away, and converted a bunch of platforms in > drivers/crypto (build tested only, except for the virtio driver) > > crypto: qce - switch to skcipher API > crypto: rockchip - switch to skcipher API > crypto: stm32 - switch to skcipher API > crypto: sahara - switch to skcipher API > crypto: picoxcell - switch to skcipher API > crypto: mediatek - switch to skcipher API > crypto: mxs - switch to skcipher API > crypto: ixp4xx - switch to skcipher API > crypto: hifn - switch to skcipher API > crypto: chelsio - switch to skcipher API > crypto: cavium/cpt - switch to skcipher API > crypto: nitrox - remove cra_type reference to ablkcipher > crypto: bcm-spu - switch to skcipher API > crypto: atmel-tdes - switch to skcipher API > crypto: atmel-aes - switch to skcipher API > crypto: s5p - switch to skcipher API > crypto: ux500 - switch to skcipher API > crypto: omap - switch to skcipher API > crypto: virtio - switch to skcipher API > crypto: virtio - deal with unsupported input sizes > crypto: virtio - implement missing support for output IVs > crypto: ccp - switch from ablkcipher to skcipher > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=ablkcipher-removal > > I pushed the branch to kernelci, so hopefully we'll get some automated > results, but I think only a small subset of these are boot tested atm. Awesome, thanks for doing this! I was just planning to do "blkcipher" for now, but your patches will take care of almost all of "ablkcipher" too. A few things I noticed from quickly skimming through your patches: "ecb-des3-omap", "cbc-des3-omap", "atmel-ecb-tdes", "atmel-cbc-tdes", and "atmel-ofb-tdes" had their min and/or max key size incorrectly changed to 8 (DES_BLOCK_SIZE or DES3_EDE_BLOCK_SIZE) rather than left as 24 (DES3_EDE_KEY_SIZE or 3*DES_KEY_SIZE). cra_blocksize for "atmel-cfb64-aes" was changed from CFB64_BLOCK_SIZE to AES_BLOCKSIZE. Intentional? cra_blocksize for "stm32-ctr-aes" and for "cfb-aes-mtk" was changed from 1 to AES_BLOCK_SIZE. Intentional? CRYPTO_ALG_NEED_FALLBACK was added to "cbc-des-picoxcell" and "ecb-des-picoxcell". Intentional? In drivers/crypto/ixp4xx_crypto.c, .walksize was set on "rfc3686(ctr(aes))" rather than .chunksize. Intentional? In drivers/crypto/qce/, CRYPTO_ALG_TYPE_ABLKCIPHER should be replaced with CRYPTO_ALG_TYPE_SKCIPHER. In drivers/crypto/stm32/, could rename crypto_algs[] to skcipher_algs[]. Thanks! - Eric
[PATCH 1/4] crypto: nx - don't abuse blkcipher_desc to pass iv around
From: Eric Biggers
The NX crypto driver is using 'struct blkcipher_desc' to pass the IV
around, even for AEADs (for which it creates the struct on the stack).
This is not appropriate since this structure is part of the "blkcipher"
API, which is deprecated and will be removed.
Just pass around the IV directly instead.
Signed-off-by: Eric Biggers
---
drivers/crypto/nx/nx-aes-cbc.c | 5 +++--
drivers/crypto/nx/nx-aes-ccm.c | 40 --
drivers/crypto/nx/nx-aes-ctr.c | 5 +++--
drivers/crypto/nx/nx-aes-ecb.c | 4 ++--
drivers/crypto/nx/nx-aes-gcm.c | 24 +---
drivers/crypto/nx/nx.c | 16 +++---
drivers/crypto/nx/nx.h | 6 ++---
7 files changed, 43 insertions(+), 57 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index e631f9979127..482a203a9260 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -72,8 +72,9 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
do {
to_process = nbytes - processed;
- rc = nx_build_sg_lists(nx_ctx, desc, dst, src, _process,
- processed, csbcpb->cpb.aes_cbc.iv);
+ rc = nx_build_sg_lists(nx_ctx, desc->info, dst, src,
+ _process, processed,
+ csbcpb->cpb.aes_cbc.iv);
if (rc)
goto out;
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 5be8f01c5da8..84fed736ed2e 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -327,7 +327,7 @@ static int generate_pat(u8 *iv,
}
static int ccm_nx_decrypt(struct aead_request *req,
- struct blkcipher_desc *desc,
+ u8*iv,
unsigned int assoclen)
{
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req->base.tfm);
@@ -348,7 +348,7 @@ static int ccm_nx_decrypt(struct aead_request *req,
req->src, nbytes + req->assoclen, authsize,
SCATTERWALK_FROM_SG);
- rc = generate_pat(desc->info, req, nx_ctx, authsize, nbytes, assoclen,
+ rc = generate_pat(iv, req, nx_ctx, authsize, nbytes, assoclen,
csbcpb->cpb.aes_ccm.in_pat_or_b0);
if (rc)
goto out;
@@ -367,7 +367,7 @@ static int ccm_nx_decrypt(struct aead_request *req,
NX_CPB_FDM(nx_ctx->csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
- rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src,
+ rc = nx_build_sg_lists(nx_ctx, iv, req->dst, req->src,
_process, processed + req->assoclen,
csbcpb->cpb.aes_ccm.iv_or_ctr);
if (rc)
@@ -381,7 +381,7 @@ static int ccm_nx_decrypt(struct aead_request *req,
/* for partial completion, copy following for next
* entry into loop...
*/
- memcpy(desc->info, csbcpb->cpb.aes_ccm.out_ctr, AES_BLOCK_SIZE);
+ memcpy(iv, csbcpb->cpb.aes_ccm.out_ctr, AES_BLOCK_SIZE);
memcpy(csbcpb->cpb.aes_ccm.in_pat_or_b0,
csbcpb->cpb.aes_ccm.out_pat_or_mac, AES_BLOCK_SIZE);
memcpy(csbcpb->cpb.aes_ccm.in_s0,
@@ -405,7 +405,7 @@ static int ccm_nx_decrypt(struct aead_request *req,
}
static int ccm_nx_encrypt(struct aead_request *req,
- struct blkcipher_desc *desc,
+ u8*iv,
unsigned int assoclen)
{
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req->base.tfm);
@@ -418,7 +418,7 @@ static int ccm_nx_encrypt(struct aead_request *req,
spin_lock_irqsave(_ctx->lock, irq_flags);
- rc = generate_pat(desc->info, req, nx_ctx, authsize, nbytes, assoclen,
+ rc = generate_pat(iv, req, nx_ctx, authsize, nbytes, assoclen,
csbcpb->cpb.aes_ccm.in_pat_or_b0);
if (rc)
goto out;
@@ -436,7 +436,7 @@ static int ccm_nx_encrypt(struct aead_request *req,
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
- rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src,
+ rc = nx_build_sg_lists(nx_ctx, iv, req->dst, req->src,
_process, processed + req->assoclen,
csbcpb->cpb.aes_ccm.iv_or_ctr);
if (rc)
@@ -450,7 +450,7 @@ static int ccm_nx_encrypt(struct aead_request *req,
/* for partial completion, copy following for next
* entry into
[PATCH 4/4] crypto: nx - convert AES-CTR to skcipher API
From: Eric Biggers
Convert the PowerPC Nest (NX) implementation of AES-CTR from the
deprecated "blkcipher" API to the "skcipher" API. This is needed in
order for the blkcipher API to be removed.
Signed-off-by: Eric Biggers
---
drivers/crypto/nx/nx-aes-ctr.c | 84 +++---
drivers/crypto/nx/nx.c | 25 +++---
drivers/crypto/nx/nx.h | 4 +-
3 files changed, 46 insertions(+), 67 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index 05e558cefe94..6d5ce1a66f1e 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -19,11 +19,11 @@
#include "nx.h"
-static int ctr_aes_nx_set_key(struct crypto_tfm *tfm,
- const u8 *in_key,
- unsigned int key_len)
+static int ctr_aes_nx_set_key(struct crypto_skcipher *tfm,
+ const u8 *in_key,
+ unsigned intkey_len)
{
- struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(tfm);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
nx_ctx_init(nx_ctx, HCOP_FC_AES);
@@ -51,11 +51,11 @@ static int ctr_aes_nx_set_key(struct crypto_tfm *tfm,
return 0;
}
-static int ctr3686_aes_nx_set_key(struct crypto_tfm *tfm,
- const u8 *in_key,
- unsigned int key_len)
+static int ctr3686_aes_nx_set_key(struct crypto_skcipher *tfm,
+ const u8 *in_key,
+ unsigned intkey_len)
{
- struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(tfm);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
if (key_len < CTR_RFC3686_NONCE_SIZE)
return -EINVAL;
@@ -69,12 +69,10 @@ static int ctr3686_aes_nx_set_key(struct crypto_tfm *tfm,
return ctr_aes_nx_set_key(tfm, in_key, key_len);
}
-static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int ctr_aes_nx_crypt(struct skcipher_request *req, u8 *iv)
{
- struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
unsigned long irq_flags;
unsigned int processed = 0, to_process;
@@ -83,9 +81,9 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
spin_lock_irqsave(_ctx->lock, irq_flags);
do {
- to_process = nbytes - processed;
+ to_process = req->cryptlen - processed;
- rc = nx_build_sg_lists(nx_ctx, desc->info, dst, src,
+ rc = nx_build_sg_lists(nx_ctx, iv, req->dst, req->src,
_process, processed,
csbcpb->cpb.aes_ctr.iv);
if (rc)
@@ -97,59 +95,51 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
}
rc = nx_hcall_sync(nx_ctx, _ctx->op,
- desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+ req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
if (rc)
goto out;
- memcpy(desc->info, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+ memcpy(iv, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
atomic_inc(&(nx_ctx->stats->aes_ops));
atomic64_add(csbcpb->csb.processed_byte_count,
&(nx_ctx->stats->aes_bytes));
processed += to_process;
- } while (processed < nbytes);
+ } while (processed < req->cryptlen);
out:
spin_unlock_irqrestore(_ctx->lock, irq_flags);
return rc;
}
-static int ctr3686_aes_nx_crypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int ctr3686_aes_nx_crypt(struct skcipher_request *req)
{
- struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
u8 iv[16];
memcpy(iv, nx_ctx->priv.ctr.nonce, CTR_RFC3686_IV_SIZE);
- memcpy(iv + CTR_RFC3686_NONCE_SIZE,
- desc->info, CTR_RFC3686_IV_SIZE);
+ memcpy(iv + CTR
[PATCH 0/4] crypto: nx - convert to skcipher API
This series converts the PowerPC Nest (NX) implementations of AES modes from the deprecated "blkcipher" API to the "skcipher" API. This is needed in order for the blkcipher API to be removed. This patchset is compile-tested only, as I don't have this hardware. If anyone has this hardware, please test this patchset with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y. Eric Biggers (4): crypto: nx - don't abuse blkcipher_desc to pass iv around crypto: nx - convert AES-ECB to skcipher API crypto: nx - convert AES-CBC to skcipher API crypto: nx - convert AES-CTR to skcipher API drivers/crypto/nx/nx-aes-cbc.c | 81 ++- drivers/crypto/nx/nx-aes-ccm.c | 40 ++-- drivers/crypto/nx/nx-aes-ctr.c | 87 +++--- drivers/crypto/nx/nx-aes-ecb.c | 76 + drivers/crypto/nx/nx-aes-gcm.c | 24 -- drivers/crypto/nx/nx.c | 64 ++--- drivers/crypto/nx/nx.h | 19 7 files changed, 176 insertions(+), 215 deletions(-) -- 2.23.0
[PATCH 3/4] crypto: nx - convert AES-CBC to skcipher API
From: Eric Biggers
Convert the PowerPC Nest (NX) implementation of AES-CBC from the
deprecated "blkcipher" API to the "skcipher" API. This is needed in
order for the blkcipher API to be removed.
Signed-off-by: Eric Biggers
---
drivers/crypto/nx/nx-aes-cbc.c | 78 ++
drivers/crypto/nx/nx.c | 11 ++---
drivers/crypto/nx/nx.h | 4 +-
3 files changed, 41 insertions(+), 52 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 482a203a9260..92e921eceed7 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -18,11 +18,11 @@
#include "nx.h"
-static int cbc_aes_nx_set_key(struct crypto_tfm *tfm,
- const u8 *in_key,
- unsigned int key_len)
+static int cbc_aes_nx_set_key(struct crypto_skcipher *tfm,
+ const u8 *in_key,
+ unsigned intkey_len)
{
- struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(tfm);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
nx_ctx_init(nx_ctx, HCOP_FC_AES);
@@ -50,13 +50,11 @@ static int cbc_aes_nx_set_key(struct crypto_tfm *tfm,
return 0;
}
-static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes,
- intenc)
+static int cbc_aes_nx_crypt(struct skcipher_request *req,
+ int enc)
{
- struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
unsigned long irq_flags;
unsigned int processed = 0, to_process;
@@ -70,9 +68,9 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
do {
- to_process = nbytes - processed;
+ to_process = req->cryptlen - processed;
- rc = nx_build_sg_lists(nx_ctx, desc->info, dst, src,
+ rc = nx_build_sg_lists(nx_ctx, req->iv, req->dst, req->src,
_process, processed,
csbcpb->cpb.aes_cbc.iv);
if (rc)
@@ -84,56 +82,46 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
}
rc = nx_hcall_sync(nx_ctx, _ctx->op,
- desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+ req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
if (rc)
goto out;
- memcpy(desc->info, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+ memcpy(req->iv, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
atomic_inc(&(nx_ctx->stats->aes_ops));
atomic64_add(csbcpb->csb.processed_byte_count,
&(nx_ctx->stats->aes_bytes));
processed += to_process;
- } while (processed < nbytes);
+ } while (processed < req->cryptlen);
out:
spin_unlock_irqrestore(_ctx->lock, irq_flags);
return rc;
}
-static int cbc_aes_nx_encrypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int cbc_aes_nx_encrypt(struct skcipher_request *req)
{
- return cbc_aes_nx_crypt(desc, dst, src, nbytes, 1);
+ return cbc_aes_nx_crypt(req, 1);
}
-static int cbc_aes_nx_decrypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int cbc_aes_nx_decrypt(struct skcipher_request *req)
{
- return cbc_aes_nx_crypt(desc, dst, src, nbytes, 0);
+ return cbc_aes_nx_crypt(req, 0);
}
-struct crypto_alg nx_cbc_aes_alg = {
- .cra_name= "cbc(aes)",
- .cra_driver_name = "cbc-aes-nx",
- .cra_priority= 300,
- .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
- .cra_blocksize = AES_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct nx_crypto_ctx),
- .cra_type= _blkcipher_type,
- .cra_alignmask = 0xf,
- .cra_module = THIS_MODULE,
- .cra_init= nx_crypto_ctx_aes_cbc_init,
-
[PATCH 2/4] crypto: nx - convert AES-ECB to skcipher API
From: Eric Biggers
Convert the PowerPC Nest (NX) implementation of AES-ECB from the
deprecated "blkcipher" API to the "skcipher" API. This is needed in
order for the blkcipher API to be removed.
Signed-off-by: Eric Biggers
---
drivers/crypto/nx/nx-aes-ecb.c | 76 ++
drivers/crypto/nx/nx.c | 28 ++---
drivers/crypto/nx/nx.h | 5 ++-
3 files changed, 58 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index 87183890d1ab..77e338dc33f1 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -18,11 +18,11 @@
#include "nx.h"
-static int ecb_aes_nx_set_key(struct crypto_tfm *tfm,
- const u8 *in_key,
- unsigned int key_len)
+static int ecb_aes_nx_set_key(struct crypto_skcipher *tfm,
+ const u8 *in_key,
+ unsigned intkey_len)
{
- struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(tfm);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = (struct nx_csbcpb *)nx_ctx->csbcpb;
nx_ctx_init(nx_ctx, HCOP_FC_AES);
@@ -50,13 +50,11 @@ static int ecb_aes_nx_set_key(struct crypto_tfm *tfm,
return 0;
}
-static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes,
- intenc)
+static int ecb_aes_nx_crypt(struct skcipher_request *req,
+ int enc)
{
- struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct nx_crypto_ctx *nx_ctx = crypto_skcipher_ctx(tfm);
struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
unsigned long irq_flags;
unsigned int processed = 0, to_process;
@@ -70,10 +68,10 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
do {
- to_process = nbytes - processed;
+ to_process = req->cryptlen - processed;
- rc = nx_build_sg_lists(nx_ctx, NULL, dst, src, _process,
- processed, NULL);
+ rc = nx_build_sg_lists(nx_ctx, NULL, req->dst, req->src,
+ _process, processed, NULL);
if (rc)
goto out;
@@ -83,7 +81,7 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
}
rc = nx_hcall_sync(nx_ctx, _ctx->op,
- desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+ req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
if (rc)
goto out;
@@ -92,46 +90,36 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
&(nx_ctx->stats->aes_bytes));
processed += to_process;
- } while (processed < nbytes);
+ } while (processed < req->cryptlen);
out:
spin_unlock_irqrestore(_ctx->lock, irq_flags);
return rc;
}
-static int ecb_aes_nx_encrypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int ecb_aes_nx_encrypt(struct skcipher_request *req)
{
- return ecb_aes_nx_crypt(desc, dst, src, nbytes, 1);
+ return ecb_aes_nx_crypt(req, 1);
}
-static int ecb_aes_nx_decrypt(struct blkcipher_desc *desc,
- struct scatterlist*dst,
- struct scatterlist*src,
- unsigned int nbytes)
+static int ecb_aes_nx_decrypt(struct skcipher_request *req)
{
- return ecb_aes_nx_crypt(desc, dst, src, nbytes, 0);
+ return ecb_aes_nx_crypt(req, 0);
}
-struct crypto_alg nx_ecb_aes_alg = {
- .cra_name= "ecb(aes)",
- .cra_driver_name = "ecb-aes-nx",
- .cra_priority= 300,
- .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
- .cra_blocksize = AES_BLOCK_SIZE,
- .cra_alignmask = 0xf,
- .cra_ctxsize = sizeof(struct nx_crypto_ctx),
- .cra_type= _blkcipher_type,
- .cra_module = THIS_MODULE,
- .cra_init= nx_crypto_ctx_aes_ecb_init,
- .cra_exit= nx_crypto_ctx_exit,
- .cra_blkcipher = {
- .min_keysize = AES_MIN_KEY_SIZE,
- .max_keysize = AES_MAX
[PATCH] crypto: powerpc - convert SPE AES algorithms to skcipher API
From: Eric Biggers
Convert the glue code for the PowerPC SPE implementations of AES-ECB,
AES-CBC, AES-CTR, and AES-XTS from the deprecated "blkcipher" API to the
"skcipher" API.
Tested with:
export ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
make mpc85xx_defconfig
cat >> .config << EOF
# CONFIG_MODULES is not set
# CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
CONFIG_DEBUG_KERNEL=y
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
CONFIG_CRYPTO_AES=y
CONFIG_CRYPTO_CBC=y
CONFIG_CRYPTO_CTR=y
CONFIG_CRYPTO_ECB=y
CONFIG_CRYPTO_XTS=y
CONFIG_CRYPTO_AES_PPC_SPE=y
EOF
make olddefconfig
make -j32
qemu-system-ppc -M mpc8544ds -cpu e500 -nographic \
-kernel arch/powerpc/boot/zImage \
-append cryptomgr.fuzz_iterations=1000
Note that xts-ppc-spe still fails the comparison tests due to the lack
of ciphertext stealing support. This is not addressed by this patch.
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/aes-spe-glue.c | 416 +
crypto/Kconfig | 1 +
2 files changed, 186 insertions(+), 231 deletions(-)
diff --git a/arch/powerpc/crypto/aes-spe-glue.c
b/arch/powerpc/crypto/aes-spe-glue.c
index 3a4ca7d32477..374e3e51e998 100644
--- a/arch/powerpc/crypto/aes-spe-glue.c
+++ b/arch/powerpc/crypto/aes-spe-glue.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
/*
@@ -86,17 +87,13 @@ static void spe_end(void)
preempt_enable();
}
-static int ppc_aes_setkey(struct crypto_tfm *tfm, const u8 *in_key,
- unsigned int key_len)
+static int expand_key(struct ppc_aes_ctx *ctx,
+ const u8 *in_key, unsigned int key_len)
{
- struct ppc_aes_ctx *ctx = crypto_tfm_ctx(tfm);
-
if (key_len != AES_KEYSIZE_128 &&
key_len != AES_KEYSIZE_192 &&
- key_len != AES_KEYSIZE_256) {
- tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+ key_len != AES_KEYSIZE_256)
return -EINVAL;
- }
switch (key_len) {
case AES_KEYSIZE_128:
@@ -114,17 +111,40 @@ static int ppc_aes_setkey(struct crypto_tfm *tfm, const
u8 *in_key,
}
ppc_generate_decrypt_key(ctx->key_dec, ctx->key_enc, key_len);
+ return 0;
+}
+static int ppc_aes_setkey(struct crypto_tfm *tfm, const u8 *in_key,
+ unsigned int key_len)
+{
+ struct ppc_aes_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ if (expand_key(ctx, in_key, key_len) != 0) {
+ tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int ppc_aes_setkey_skcipher(struct crypto_skcipher *tfm,
+ const u8 *in_key, unsigned int key_len)
+{
+ struct ppc_aes_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+ if (expand_key(ctx, in_key, key_len) != 0) {
+ crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return -EINVAL;
+ }
return 0;
}
-static int ppc_xts_setkey(struct crypto_tfm *tfm, const u8 *in_key,
+static int ppc_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
unsigned int key_len)
{
- struct ppc_xts_ctx *ctx = crypto_tfm_ctx(tfm);
+ struct ppc_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
int err;
- err = xts_check_key(tfm, in_key, key_len);
+ err = xts_verify_key(tfm, in_key, key_len);
if (err)
return err;
@@ -133,7 +153,7 @@ static int ppc_xts_setkey(struct crypto_tfm *tfm, const u8
*in_key,
if (key_len != AES_KEYSIZE_128 &&
key_len != AES_KEYSIZE_192 &&
key_len != AES_KEYSIZE_256) {
- tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+ crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
return -EINVAL;
}
@@ -178,208 +198,154 @@ static void ppc_aes_decrypt(struct crypto_tfm *tfm, u8
*out, const u8 *in)
spe_end();
}
-static int ppc_ecb_encrypt(struct blkcipher_desc *desc, struct scatterlist
*dst,
- struct scatterlist *src, unsigned int nbytes)
+static int ppc_ecb_crypt(struct skcipher_request *req, bool enc)
{
- struct ppc_aes_ctx *ctx = crypto_blkcipher_ctx(desc->tfm);
- struct blkcipher_walk walk;
- unsigned int ubytes;
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct ppc_aes_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct skcipher_walk walk;
+ unsigned int nbytes;
int err;
- desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
- blkcipher_walk_init(, dst, src, nbytes);
- err = blkcipher_walk_virt(desc, );
+ err = skcipher_walk_virt(, req, false
[PATCH] crypto: testmgr - fix length truncation with large page size
From: Eric Biggers
On PowerPC with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y, there is sometimes
a crash in generate_random_aead_testvec(). The problem is that the
generated test vectors use data lengths of up to about 2 * PAGE_SIZE,
which is 128 KiB on PowerPC; however, the data length fields in the test
vectors are 'unsigned short', so the lengths get truncated. Fix this by
changing the relevant fields to 'unsigned int'.
Fixes: 40153b10d91c ("crypto: testmgr - fuzz AEADs against their generic
implementation")
Signed-off-by: Eric Biggers
---
crypto/testmgr.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 9a13c634b2077..fb2afdd544e00 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -43,7 +43,7 @@ struct hash_testvec {
const char *key;
const char *plaintext;
const char *digest;
- unsigned short psize;
+ unsigned int psize;
unsigned short ksize;
int setkey_error;
int digest_error;
@@ -74,7 +74,7 @@ struct cipher_testvec {
const char *ctext;
unsigned char wk; /* weak key flag */
unsigned short klen;
- unsigned short len;
+ unsigned int len;
bool fips_skip;
bool generates_iv;
int setkey_error;
@@ -110,9 +110,9 @@ struct aead_testvec {
unsigned char novrfy;
unsigned char wk;
unsigned char klen;
- unsigned short plen;
- unsigned short clen;
- unsigned short alen;
+ unsigned int plen;
+ unsigned int clen;
+ unsigned int alen;
int setkey_error;
int setauthsize_error;
int crypt_error;
--
2.21.0.1020.gf2820cf01a-goog
[PATCH] crypto: vmx - convert to skcipher API
From: Eric Biggers
Convert the VMX implementations of AES-CBC, AES-CTR, and AES-XTS from
the deprecated "blkcipher" API to the "skcipher" API.
As part of this, I moved the skcipher_request for the fallback algorithm
off the stack and into the request context of the parent algorithm.
I tested this in a PowerPC VM with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.
Signed-off-by: Eric Biggers
---
drivers/crypto/vmx/aes_cbc.c | 183 -
drivers/crypto/vmx/aes_ctr.c | 165 +
drivers/crypto/vmx/aes_xts.c | 175 ++-
drivers/crypto/vmx/aesp8-ppc.h | 2 -
drivers/crypto/vmx/vmx.c | 72 +++--
5 files changed, 252 insertions(+), 345 deletions(-)
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index dae8af3c46dce..92e75a05d6a9e 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -7,64 +7,52 @@
* Author: Marcelo Henrique Cerri
*/
-#include
-#include
-#include
-#include
#include
#include
#include
#include
-#include
-#include
+#include
#include "aesp8-ppc.h"
struct p8_aes_cbc_ctx {
- struct crypto_sync_skcipher *fallback;
+ struct crypto_skcipher *fallback;
struct aes_key enc_key;
struct aes_key dec_key;
};
-static int p8_aes_cbc_init(struct crypto_tfm *tfm)
+static int p8_aes_cbc_init(struct crypto_skcipher *tfm)
{
- const char *alg = crypto_tfm_alg_name(tfm);
- struct crypto_sync_skcipher *fallback;
- struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
-
- fallback = crypto_alloc_sync_skcipher(alg, 0,
- CRYPTO_ALG_NEED_FALLBACK);
+ struct p8_aes_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct crypto_skcipher *fallback;
+ fallback = crypto_alloc_skcipher("cbc(aes)", 0,
+CRYPTO_ALG_NEED_FALLBACK |
+CRYPTO_ALG_ASYNC);
if (IS_ERR(fallback)) {
- printk(KERN_ERR
- "Failed to allocate transformation for '%s': %ld\n",
- alg, PTR_ERR(fallback));
+ pr_err("Failed to allocate cbc(aes) fallback: %ld\n",
+ PTR_ERR(fallback));
return PTR_ERR(fallback);
}
- crypto_sync_skcipher_set_flags(
- fallback,
- crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
+ crypto_skcipher_set_reqsize(tfm, sizeof(struct skcipher_request) +
+ crypto_skcipher_reqsize(fallback));
ctx->fallback = fallback;
-
return 0;
}
-static void p8_aes_cbc_exit(struct crypto_tfm *tfm)
+static void p8_aes_cbc_exit(struct crypto_skcipher *tfm)
{
- struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
+ struct p8_aes_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
- if (ctx->fallback) {
- crypto_free_sync_skcipher(ctx->fallback);
- ctx->fallback = NULL;
- }
+ crypto_free_skcipher(ctx->fallback);
}
-static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const u8 *key,
+static int p8_aes_cbc_setkey(struct crypto_skcipher *tfm, const u8 *key,
unsigned int keylen)
{
+ struct p8_aes_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
int ret;
- struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
preempt_disable();
pagefault_disable();
@@ -75,108 +63,71 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const
u8 *key,
pagefault_enable();
preempt_enable();
- ret |= crypto_sync_skcipher_setkey(ctx->fallback, key, keylen);
+ ret |= crypto_skcipher_setkey(ctx->fallback, key, keylen);
return ret ? -EINVAL : 0;
}
-static int p8_aes_cbc_encrypt(struct blkcipher_desc *desc,
- struct scatterlist *dst,
- struct scatterlist *src, unsigned int nbytes)
+static int p8_aes_cbc_crypt(struct skcipher_request *req, int enc)
{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ const struct p8_aes_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct skcipher_walk walk;
+ unsigned int nbytes;
int ret;
- struct blkcipher_walk walk;
- struct p8_aes_cbc_ctx *ctx =
- crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
if (!crypto_simd_usable()) {
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
- skcipher_request_set_sync_tfm(req, ctx->fallback);
- skcipher_request_set_callback(req, desc->flags, NULL, NULL);
- skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
- ret = crypto_skcipher_encrypt(req);
- skcipher_request_zero(req
[PATCH] crypto: vmx - convert to SPDX license identifiers
From: Eric Biggers Remove the boilerplate license text and replace it with the equivalent SPDX license identifier. Signed-off-by: Eric Biggers --- drivers/crypto/vmx/aes.c | 14 +- drivers/crypto/vmx/aes_cbc.c | 14 +- drivers/crypto/vmx/aes_ctr.c | 14 +- drivers/crypto/vmx/aes_xts.c | 14 +- drivers/crypto/vmx/vmx.c | 14 +- 5 files changed, 5 insertions(+), 65 deletions(-) diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c index 603a620819941..2e9476158df49 100644 --- a/drivers/crypto/vmx/aes.c +++ b/drivers/crypto/vmx/aes.c @@ -1,21 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /** * AES routines supporting VMX instructions on the Power 8 * * Copyright (C) 2015 International Business Machines Inc. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 only. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Author: Marcelo Henrique Cerri */ diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c index a1a9a6f0d42cf..dae8af3c46dce 100644 --- a/drivers/crypto/vmx/aes_cbc.c +++ b/drivers/crypto/vmx/aes_cbc.c @@ -1,21 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /** * AES CBC routines supporting VMX instructions on the Power 8 * * Copyright (C) 2015 International Business Machines Inc. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 only. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Author: Marcelo Henrique Cerri */ diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c index 192a53512f5e8..dc31101178446 100644 --- a/drivers/crypto/vmx/aes_ctr.c +++ b/drivers/crypto/vmx/aes_ctr.c @@ -1,21 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /** * AES CTR routines supporting VMX instructions on the Power 8 * * Copyright (C) 2015 International Business Machines Inc. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 only. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Author: Marcelo Henrique Cerri */ diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c index 00d412d811ae6..aee1339f134ec 100644 --- a/drivers/crypto/vmx/aes_xts.c +++ b/drivers/crypto/vmx/aes_xts.c @@ -1,21 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /** * AES XTS routines supporting VMX In-core instructions on Power 8 * * Copyright (C) 2015 International Business Machines Inc. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundations; version 2 only. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY of FITNESS FOR A PARTICUPAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Author: Leonidas S. Barbosa */ diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c index a9f5198306155..abd89c2bcec4d 100644 --- a/drivers/crypto/vmx/vmx.c +++ b/drivers/crypto/vmx/vmx.c @@ -1,21 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /** * Routines supporting VMX instructions
Re: [PATCH] crypto: vmx - CTR: always increment IV as quadword
On Mon, May 20, 2019 at 11:59:05AM +1000, Daniel Axtens wrote:
> Daniel Axtens writes:
>
> > The kernel self-tests picked up an issue with CTR mode:
> > alg: skcipher: p8_aes_ctr encryption test failed (wrong result) on test
> > vector 3, cfg="uneven misaligned splits, may sleep"
> >
> > Test vector 3 has an IV of FFFD, so
> > after 3 increments it should wrap around to 0.
> >
> > In the aesp8-ppc code from OpenSSL, there are two paths that
> > increment IVs: the bulk (8 at a time) path, and the individual
> > path which is used when there are fewer than 8 AES blocks to
> > process.
> >
> > In the bulk path, the IV is incremented with vadduqm: "Vector
> > Add Unsigned Quadword Modulo", which does 128-bit addition.
> >
> > In the individual path, however, the IV is incremented with
> > vadduwm: "Vector Add Unsigned Word Modulo", which instead
> > does 4 32-bit additions. Thus the IV would instead become
> > , throwing off the result.
> >
> > Use vadduqm.
> >
> > This was probably a typo originally, what with q and w being
> > adjacent. It is a pretty narrow edge case: I am really
> > impressed by the quality of the kernel self-tests!
> >
> > Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by
> > ASM")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Daniel Axtens
> >
> > ---
> >
> > I'll pass this along internally to get it into OpenSSL as well.
>
> I passed this along to OpenSSL and got pretty comprehensively schooled:
> https://github.com/openssl/openssl/pull/8942
>
> It seems we tweak the openssl code to use a 128-bit counter, whereas
> the original code was in fact designed for a 32-bit counter. We must
> have changed the vaddu instruction in the bulk path but not in the
> individual path, as they're both vadduwm (4x32-bit) upstream.
>
> I think this change is still correct with regards to the kernel,
> but I guess it's probably something where I should have done a more
> thorough read of the documentation before diving in to the code, and
> perhaps we should note it in the code somewhere too. Ah well.
>
> Regards,
> Daniel
>
Ah, I didn't realize there are multiple conventions for CTR. Yes, all CTR
implementations in the kernel follow the 128-bit convention, and evidently the
test vectors test for that. Apparently the VMX OpenSSL code got incompletely
changed from the 32-bit convention by this commit, so that's what you're fixing:
commit 1d4aa0b4c1816e8ca92a6aadb0d8f6b43c56c0d0
Author: Leonidas Da Silva Barbosa
Date: Fri Aug 14 10:12:22 2015 -0300
crypto: vmx - Fixing AES-CTR counter bug
AES-CTR is using a counter 8bytes-8bytes what miss match with
kernel specs.
In the previous code a vadduwm was done to increment counter.
Replacing this for a vadduqm now considering both cases counter
8-8 bytes and full 16bytes.
A comment in the code would indeed be helpful.
Note that the kernel doesn't currently need a 32-bit CTR implementation for GCM
like OpenSSL does, because the kernel currently only supports 12-byte IVs with
GCM. So the low 32 bits of the counter start at 1 and don't overflow,
regardless of whether the counter is incremented mod 2^32 or mod 2^128.
- Eric
Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
On Thu, May 16, 2019 at 12:12:48PM +1000, Daniel Axtens wrote: > > I'm also seeing issues with ghash with the extended tests: > > [7.582926] alg: hash: p8_ghash test failed (wrong result) on test vector > 0, cfg="random: use_final src_divs=[9.72%@+39832, > 18.2%@+65504, 45.57%@alignmask+18, > 15.6%@+65496, 6.83%@+65514, 1.2%@+25, > It seems to happen when one of the source divisions has nosimd and the > final result uses the simd finaliser, so that's interesting. > The bug is that p8_ghash uses different shash_descs for the SIMD and no-SIMD cases. So if you start out doing the hash in SIMD context but then switch to no-SIMD context or vice versa, the digest will be wrong. Note that there can be an ->export() and ->import() in between, so it's not quite as obscure a case as one might think. To fix it I think you'll need to make p8_ghash use 'struct ghash_desc_ctx' just like ghash-generic so that the two code paths can share the same shash_desc. That's similar to what the various SHA hash algorithms do. - Eric
Re: [PATCH] crypto: talitos - fix skcipher failure due to wrong output IV
On Wed, May 15, 2019 at 08:49:48PM +0200, Christophe Leroy wrote:
>
>
> Le 15/05/2019 à 16:05, Horia Geanta a écrit :
> > On 5/15/2019 3:29 PM, Christophe Leroy wrote:
> > > Selftests report the following:
> > >
> > > [2.984845] alg: skcipher: cbc-aes-talitos encryption test failed
> > > (wrong output IV) on test vector 0, cfg="in-place"
> > > [2.995377] : 3d af ba 42 9d 9e b4 30 b4 22 da 80 2c 9f ac 41
> > > [3.032673] alg: skcipher: cbc-des-talitos encryption test failed
> > > (wrong output IV) on test vector 0, cfg="in-place"
> > > [3.043185] : fe dc ba 98 76 54 32 10
> > > [3.063238] alg: skcipher: cbc-3des-talitos encryption test failed
> > > (wrong output IV) on test vector 0, cfg="in-place"
> > > [3.073818] : 7d 33 88 93 0f 93 b2 42
> > >
> > > This above dumps show that the actual output IV is indeed the input IV.
> > > This is due to the IV not being copied back into the request.
> > >
> > > This patch fixes that.
> > >
> > > Signed-off-by: Christophe Leroy
> > Reviewed-by: Horia Geantă
>
> It's missing a Fixes: tag and a Cc: to stable.
>
> I'll resend tomorrow.
>
> >
> > While here, could you please check ecb mode (which by definition does not
> > have
> > an IV) is behaving correctly?
> > Looking in driver_algs[] list of crypto algorithms supported by talitos,
> > ecb(aes,des,3des) are declared with ivsize != 0.
>
> According to /proc/crypto, test are passed for ecb.
>
Did you try enabling CONFIG_CRYPTO_MANAGER_EXTRA_TESTS? There is now a check
that the driver's ivsize matches the generic implementation's:
if (ivsize != crypto_skcipher_ivsize(generic_tfm)) {
pr_err("alg: skcipher: ivsize for %s (%u) doesn't match generic
impl (%u)\n",
driver, ivsize, crypto_skcipher_ivsize(generic_tfm));
err = -EINVAL;
goto out;
}
For ECB that means the ivsize must be 0.
AFAICS the talitos driver even accesses the IV for ECB, which is wrong; and the
only reason this isn't crashing the self-tests already is that they are confused
by the declared ivsize being nonzero so they don't pass NULL as they should.
- Eric
[RFC PATCH 2/3] crypto: nx - don't abuse shash MAY_SLEEP flag
From: Eric Biggers The nx driver uses the MAY_SLEEP flag in shash_desc::flags as an indicator to not retry sending the operation to the hardware as many times before returning -EBUSY. This is bogus because (1) that's not what the MAY_SLEEP flag is for, and (2) the shash API doesn't allow failing if the hardware is busy anyway. For now, just make it always retry the larger number of times. This doesn't actually fix this driver, but it at least makes it not use the shash_desc::flags field anymore. Then this field can be removed, as no other drivers use it. Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Eric Biggers --- drivers/crypto/nx/nx-aes-xcbc.c | 12 drivers/crypto/nx/nx-sha256.c | 6 ++ drivers/crypto/nx/nx-sha512.c | 6 ++ 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c index ad3358e74f5c4..8f5820b78a83b 100644 --- a/drivers/crypto/nx/nx-aes-xcbc.c +++ b/drivers/crypto/nx/nx-aes-xcbc.c @@ -105,8 +105,7 @@ static int nx_xcbc_empty(struct shash_desc *desc, u8 *out) nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg); nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg); - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; atomic_inc(&(nx_ctx->stats->aes_ops)); @@ -134,8 +133,7 @@ static int nx_xcbc_empty(struct shash_desc *desc, u8 *out) nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg); nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg); - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; atomic_inc(&(nx_ctx->stats->aes_ops)); @@ -279,8 +277,7 @@ static int nx_xcbc_update(struct shash_desc *desc, goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; @@ -361,8 +358,7 @@ static int nx_xcbc_final(struct shash_desc *desc, u8 *out) goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c index a6764af83c6dc..e06f0431dee5f 100644 --- a/drivers/crypto/nx/nx-sha256.c +++ b/drivers/crypto/nx/nx-sha256.c @@ -162,8 +162,7 @@ static int nx_sha256_update(struct shash_desc *desc, const u8 *data, goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; @@ -243,8 +242,7 @@ static int nx_sha256_final(struct shash_desc *desc, u8 *out) goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c index 92956bc6e45ee..0293b17903d0c 100644 --- a/drivers/crypto/nx/nx-sha512.c +++ b/drivers/crypto/nx/nx-sha512.c @@ -166,8 +166,7 @@ static int nx_sha512_update(struct shash_desc *desc, const u8 *data, goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; @@ -249,8 +248,7 @@ static int nx_sha512_final(struct shash_desc *desc, u8 *out) goto out; } - rc = nx_hcall_sync(nx_ctx, _ctx->op, - desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP); + rc = nx_hcall_sync(nx_ctx, _ctx->op, 0); if (rc) goto out; -- 2.21.0
[PATCH] crypto: powerpc - convert to use crypto_simd_usable()
From: Eric Biggers
Replace all calls to in_interrupt() in the PowerPC crypto code with
!crypto_simd_usable(). This causes the crypto self-tests to test the
no-SIMD code paths when CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.
The p8_ghash algorithm is currently failing and needs to be fixed, as it
produces the wrong digest when no-SIMD updates are mixed with SIMD ones.
Signed-off-by: Eric Biggers
---
arch/powerpc/crypto/crc32c-vpmsum_glue.c| 4 +++-
arch/powerpc/crypto/crct10dif-vpmsum_glue.c | 4 +++-
arch/powerpc/include/asm/Kbuild | 1 +
drivers/crypto/vmx/aes.c| 7 ---
drivers/crypto/vmx/aes_cbc.c| 7 ---
drivers/crypto/vmx/aes_ctr.c| 5 +++--
drivers/crypto/vmx/aes_xts.c| 5 +++--
drivers/crypto/vmx/ghash.c | 9 -
8 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/crypto/crc32c-vpmsum_glue.c
b/arch/powerpc/crypto/crc32c-vpmsum_glue.c
index fd1d6c83f0c02..c4fa242dd652d 100644
--- a/arch/powerpc/crypto/crc32c-vpmsum_glue.c
+++ b/arch/powerpc/crypto/crc32c-vpmsum_glue.c
@@ -1,10 +1,12 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#define CHKSUM_BLOCK_SIZE 1
@@ -22,7 +24,7 @@ static u32 crc32c_vpmsum(u32 crc, unsigned char const *p,
size_t len)
unsigned int prealign;
unsigned int tail;
- if (len < (VECTOR_BREAKPOINT + VMX_ALIGN) || in_interrupt())
+ if (len < (VECTOR_BREAKPOINT + VMX_ALIGN) || !crypto_simd_usable())
return __crc32c_le(crc, p, len);
if ((unsigned long)p & VMX_ALIGN_MASK) {
diff --git a/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
b/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
index 02ea277863d15..e27ff16573b5b 100644
--- a/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
+++ b/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
@@ -12,11 +12,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#define VMX_ALIGN 16
@@ -32,7 +34,7 @@ static u16 crct10dif_vpmsum(u16 crci, unsigned char const *p,
size_t len)
unsigned int tail;
u32 crc = crci;
- if (len < (VECTOR_BREAKPOINT + VMX_ALIGN) || in_interrupt())
+ if (len < (VECTOR_BREAKPOINT + VMX_ALIGN) || !crypto_simd_usable())
return crc_t10dif_generic(crc, p, len);
if ((unsigned long)p & VMX_ALIGN_MASK) {
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index a0c132bedfae8..5ac3dead69523 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -11,3 +11,4 @@ generic-y += preempt.h
generic-y += rwsem.h
generic-y += vtime.h
generic-y += msi.h
+generic-y += simd.h
diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
index b00d6947e02f4..603a620819941 100644
--- a/drivers/crypto/vmx/aes.c
+++ b/drivers/crypto/vmx/aes.c
@@ -23,9 +23,10 @@
#include
#include
#include
-#include
+#include
#include
#include
+#include
#include "aesp8-ppc.h"
@@ -92,7 +93,7 @@ static void p8_aes_encrypt(struct crypto_tfm *tfm, u8 *dst,
const u8 *src)
{
struct p8_aes_ctx *ctx = crypto_tfm_ctx(tfm);
- if (in_interrupt()) {
+ if (!crypto_simd_usable()) {
crypto_cipher_encrypt_one(ctx->fallback, dst, src);
} else {
preempt_disable();
@@ -109,7 +110,7 @@ static void p8_aes_decrypt(struct crypto_tfm *tfm, u8 *dst,
const u8 *src)
{
struct p8_aes_ctx *ctx = crypto_tfm_ctx(tfm);
- if (in_interrupt()) {
+ if (!crypto_simd_usable()) {
crypto_cipher_decrypt_one(ctx->fallback, dst, src);
} else {
preempt_disable();
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index fbe882ef1bc5d..a1a9a6f0d42cf 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -23,9 +23,10 @@
#include
#include
#include
-#include
+#include
#include
#include
+#include
#include
#include
@@ -100,7 +101,7 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc *desc,
struct p8_aes_cbc_ctx *ctx =
crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
- if (in_interrupt()) {
+ if (!crypto_simd_usable()) {
SYNC_SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
skcipher_request_set_sync_tfm(req, ctx->fallback);
skcipher_request_set_callback(req, desc->flags, NULL, NULL);
@@ -139,7 +140,7 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc *desc,
struct p8_aes_cbc_ctx *ctx =
crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
- if (in_interrupt()) {
+ if (!crypto_simd_usable()) {
SYNC_SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
Hi Daniel, On Fri, Mar 15, 2019 at 04:23:02PM +1100, Daniel Axtens wrote: > Eric Biggers writes: > > > Hi Daniel, > > > > On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote: > >> Hi Eric, > >> > >> >> The original assembly imported from OpenSSL has two copy-paste > >> >> errors in handling CTR mode. When dealing with a 2 or 3 block tail, > >> >> the code branches to the CBC decryption exit path, rather than to > >> >> the CTR exit path. > >> > > >> > So does this need to be fixed in OpenSSL too? > >> > >> Yes, I'm getting in touch with some people internally (at IBM) about > >> doing that. > >> > >> >> This leads to corruption of the IV, which leads to subsequent blocks > >> >> being corrupted. > >> >> > >> >> This can be detected with libkcapi test suite, which is available at > >> >> https://github.com/smuellerDD/libkcapi > >> >> > >> > > >> > Is this also detected by the kernel's crypto self-tests, and if not why > >> > not? > >> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y? > >> > >> It seems the self-tests do not catch it. To catch it, there has to be a > >> test where the blkcipher_walk creates a walk.nbytes such that > >> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with > >> AF_ALG pretty frequently, but when I booted with self-tests it only hit > >> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3. > >> > >> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in > >> -next? > >> > >> Regards, > >> Daniel > > > > The improvements I recently made to the self-tests are intended to catch > > exactly > > this sort of bug. They were just merged for v5.1, so try the latest > > mainline. > > This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to > > know), but it may be caught by the regular self-tests now too. > > Well, even the patched code fails with the new self-tests, so clearly > they're catching something! I'll investigate in more detail next week. > > Regards, > Daniel > > > > > - Eric Are you still planning to fix the remaining bug? I booted a ppc64le VM, and I see the same test failure (I think) you were referring to: alg: skcipher: p8_aes_ctr encryption test failed (wrong result) on test vector 3, cfg="uneven misaligned splits, may sleep" - Eric
Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
Hi Daniel, On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote: > Hi Eric, > > >> The original assembly imported from OpenSSL has two copy-paste > >> errors in handling CTR mode. When dealing with a 2 or 3 block tail, > >> the code branches to the CBC decryption exit path, rather than to > >> the CTR exit path. > > > > So does this need to be fixed in OpenSSL too? > > Yes, I'm getting in touch with some people internally (at IBM) about > doing that. > > >> This leads to corruption of the IV, which leads to subsequent blocks > >> being corrupted. > >> > >> This can be detected with libkcapi test suite, which is available at > >> https://github.com/smuellerDD/libkcapi > >> > > > > Is this also detected by the kernel's crypto self-tests, and if not why not? > > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y? > > It seems the self-tests do not catch it. To catch it, there has to be a > test where the blkcipher_walk creates a walk.nbytes such that > [(the number of AES blocks) mod 8] is either 2 or 3. This happens with > AF_ALG pretty frequently, but when I booted with self-tests it only hit > 1, 4, 5, 6 and 7 - it missed 0, 2 and 3. > > I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in > -next? > > Regards, > Daniel The improvements I recently made to the self-tests are intended to catch exactly this sort of bug. They were just merged for v5.1, so try the latest mainline. This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to know), but it may be caught by the regular self-tests now too. - Eric
Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
Hi Daniel,
On Fri, Mar 15, 2019 at 01:09:01PM +1100, Daniel Axtens wrote:
> The original assembly imported from OpenSSL has two copy-paste
> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> the code branches to the CBC decryption exit path, rather than to
> the CTR exit path.
So does this need to be fixed in OpenSSL too?
>
> This leads to corruption of the IV, which leads to subsequent blocks
> being corrupted.
>
> This can be detected with libkcapi test suite, which is available at
> https://github.com/smuellerDD/libkcapi
>
Is this also detected by the kernel's crypto self-tests, and if not why not?
What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
> Reported-by: Ondrej Mosnáček
> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by ASM")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Daniel Axtens
> ---
> drivers/crypto/vmx/aesp8-ppc.pl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-ppc.pl
> index d6a9f63d65ba..de78282b8f44 100644
> --- a/drivers/crypto/vmx/aesp8-ppc.pl
> +++ b/drivers/crypto/vmx/aesp8-ppc.pl
> @@ -1854,7 +1854,7 @@ Lctr32_enc8x_three:
> stvx_u $out1,$x10,$out
> stvx_u $out2,$x20,$out
> addi$out,$out,0x30
> - b Lcbc_dec8x_done
> + b Lctr32_enc8x_done
>
> .align 5
> Lctr32_enc8x_two:
> @@ -1866,7 +1866,7 @@ Lctr32_enc8x_two:
> stvx_u $out0,$x00,$out
> stvx_u $out1,$x10,$out
> addi$out,$out,0x20
> - b Lcbc_dec8x_done
> + b Lctr32_enc8x_done
>
> .align 5
> Lctr32_enc8x_one:
> --
> 2.19.1
>
Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial
Hi Krzysztof, On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote: > Hi, > > Kernel defines same polynomial for CRC-32 in few places. > This is unnecessary duplication of the same value. Also this might > be error-prone for future code - every driver will define the > polynomial again. > > This is an attempt to unify definition of polynomial. Few obvious > hard-coded locations are fixed with define. > > All series depend on each 1/6 and 2/6. > > This could be merged in two different merge windows (1st lib/crc and then > the rest) or taken through one tree. > > It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 > and Freescale's Ethernet driver were tested on HW. Rest got just different > builds. > > Best regards, > Krzysztof > > Krzysztof Kozlowski (6): > lib/crc: Move polynomial definition to separate header > lib/crc: Use consistent naming for CRC-32 polynomials > crypto: stm32_crc32 - Use existing define with polynomial > net: ethernet: Use existing define with polynomial > staging: rtl: Use existing define with polynomial > lib: Use existing define with polynomial > > drivers/crypto/stm32/stm32_crc32.c | 11 --- > drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++-- > drivers/net/ethernet/apple/bmac.c| 8 ++-- > drivers/net/ethernet/broadcom/tg3.c | 3 ++- > drivers/net/ethernet/freescale/fec_main.c| 4 ++-- > drivers/net/ethernet/freescale/fs_enet/fec.h | 3 --- > drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++- > drivers/net/ethernet/micrel/ks8851_mll.c | 3 ++- > drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c| 4 ++-- > drivers/staging/rtl8712/rtl871x_security.c | 5 ++--- > drivers/staging/rtl8723bs/core/rtw_security.c| 5 ++--- > include/linux/crc32poly.h| 20 > lib/crc32.c | 11 ++- > lib/crc32defs.h | 14 -- > lib/decompress_bunzip2.c | 3 ++- > lib/gen_crc32table.c | 5 +++-- > lib/xz/xz_crc32.c| 3 ++- > 17 files changed, 55 insertions(+), 54 deletions(-) > create mode 100644 include/linux/crc32poly.h > Did you check whether any of these users can be converted to use the CRC implementations in lib/, so they wouldn't need the polynomial definition themselves? - Eric
