* Re: [PATCH 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-21 13:04 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-ia64, linux-sh, linux-kernel, dhowells, linux-kselftest,
sparclinux, shuah, linux-arch, linux-s390, miklos, x86, torvalds,
linux-mips, linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro,
tglx, ldv, linux-arm-kernel, linux-parisc, linux-api, oleg,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <87tvdoau12.fsf@oldenburg2.str.redhat.com>
On Tue, May 21, 2019 at 02:09:29PM +0200, Florian Weimer wrote:
> * Christian Brauner:
>
> > +/**
> > + * __close_range() - Close all file descriptors in a given range.
> > + *
> > + * @fd: starting file descriptor to close
> > + * @max_fd: last file descriptor to close
> > + *
> > + * This closes a range of file descriptors. All file descriptors
> > + * from @fd up to and including @max_fd are closed.
> > + */
> > +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> > +{
> > + unsigned int cur_max;
> > +
> > + if (fd > max_fd)
> > + return -EINVAL;
> > +
> > + rcu_read_lock();
> > + cur_max = files_fdtable(files)->max_fds;
> > + rcu_read_unlock();
> > +
> > + /* cap to last valid index into fdtable */
> > + if (max_fd >= cur_max)
> > + max_fd = cur_max - 1;
> > +
> > + while (fd <= max_fd)
> > + __close_fd(files, fd++);
> > +
> > + return 0;
> > +}
>
> This seems rather drastic. How long does this block in kernel mode?
> Maybe it's okay as long as the maximum possible value for cur_max stays
> around 4 million or so.
That's probably valid concern when you reach very high numbers though I
wonder how relevant this is in practice.
Also, you would only be blocking yourself I imagine, i.e. you can't DOS
another task with this unless your multi-threaded.
>
> Solaris has an fdwalk function:
>
> <https://docs.oracle.com/cd/E88353_01/html/E37843/closefrom-3c.html>
>
> So a different way to implement this would expose a nextfd system call
Meh. If nextfd() then I would like it to be able to:
- get the nextfd(fd) >= fd
- get highest open fd e.g. nextfd(-1)
But then I wonder if nextfd() needs to be a syscall and isn't just
either:
fcntl(fd, F_GET_NEXT)?
or
prctl(PR_GET_NEXT)?
Technically, one could also do:
fd_range(unsigned fd, unsigend end_fd, unsigned flags);
fd_range(3, 50, FD_RANGE_CLOSE);
/* return highest fd within the range [3, 50] */
fd_range(3, 50, FD_RANGE_NEXT);
/* return highest fd */
fd_range(3, UINT_MAX, FD_RANGE_NEXT);
This syscall could also reasonably be extended.
> to userspace, so that we can use that to implement both fdwalk and
> closefrom. But maybe fdwalk is just too obscure, given the existence of
> /proc.
Yeah we probably don't need fdwalk.
>
> I'll happily implement closefrom on top of close_range in glibc (plus
> fallback for older kernels based on /proc—with an abort in case that
> doesn't work because the RLIMIT_NOFILE hack is unreliable
> unfortunately).
>
> Thanks,
> Florian
^ permalink raw reply
* Re: [PATCH 1/2] open: add close_range()
From: Florian Weimer @ 2019-05-21 13:10 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-ia64, linux-sh, linux-kernel, dhowells, linux-kselftest,
sparclinux, shuah, linux-arch, linux-s390, miklos, x86, torvalds,
linux-mips, linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro,
tglx, ldv, linux-arm-kernel, linux-parisc, linux-api, oleg,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190521130438.q3u4wvve7p6md6cm@brauner.io>
* Christian Brauner:
>> Solaris has an fdwalk function:
>>
>> <https://docs.oracle.com/cd/E88353_01/html/E37843/closefrom-3c.html>
>>
>> So a different way to implement this would expose a nextfd system call
>
> Meh. If nextfd() then I would like it to be able to:
> - get the nextfd(fd) >= fd
> - get highest open fd e.g. nextfd(-1)
The highest open descriptor isn't istering for fdwalk because nextfd
would just fail.
> But then I wonder if nextfd() needs to be a syscall and isn't just
> either:
> fcntl(fd, F_GET_NEXT)?
> or
> prctl(PR_GET_NEXT)?
I think the fcntl route is a bit iffy because you might need it to get
the *first* valid descriptor.
>> to userspace, so that we can use that to implement both fdwalk and
>> closefrom. But maybe fdwalk is just too obscure, given the existence of
>> /proc.
>
> Yeah we probably don't need fdwalk.
Agreed. Just wanted to bring it up for completeness. I certainly don't
want to derail the implementation of close_range.
Thanks,
Florian
^ permalink raw reply
* [PATCH v2] powerpc/mm: mark more tlb functions as __always_inline
From: Masahiro Yamada @ 2019-05-21 13:13 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: linux-kernel, Nicholas Piggin, Masahiro Yamada, Paul Mackerras,
Aneesh Kumar K.V, Suraj Jitindar Singh, Andrew Morton,
David Gibson
With CONFIG_OPTIMIZE_INLINING enabled, Laura Abbott reported error
with gcc 9.1.1:
arch/powerpc/mm/book3s64/radix_tlb.c: In function '_tlbiel_pid':
arch/powerpc/mm/book3s64/radix_tlb.c:104:2: warning: asm operand 3 probably doesn't match constraints
104 | asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
| ^~~
arch/powerpc/mm/book3s64/radix_tlb.c:104:2: error: impossible constraint in 'asm'
Fixing _tlbiel_pid() is enough to address the warning above, but I
inlined more functions to fix all potential issues.
To meet the "i" (immediate) constraint for the asm operands, functions
propagating "ric" must be always inlined.
Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
Reported-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v2:
- Do not split lines
arch/powerpc/mm/book3s64/hash_native.c | 2 +-
arch/powerpc/mm/book3s64/radix_tlb.c | 32 ++++++++++++++++----------------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
index aaa28fd..c854151 100644
--- a/arch/powerpc/mm/book3s64/hash_native.c
+++ b/arch/powerpc/mm/book3s64/hash_native.c
@@ -60,7 +60,7 @@ static inline void tlbiel_hash_set_isa206(unsigned int set, unsigned int is)
* tlbiel instruction for hash, set invalidation
* i.e., r=1 and is=01 or is=10 or is=11
*/
-static inline void tlbiel_hash_set_isa300(unsigned int set, unsigned int is,
+static __always_inline void tlbiel_hash_set_isa300(unsigned int set, unsigned int is,
unsigned int pid,
unsigned int ric, unsigned int prs)
{
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index 4d84136..4d3dc10 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -29,7 +29,7 @@
* tlbiel instruction for radix, set invalidation
* i.e., r=1 and is=01 or is=10 or is=11
*/
-static inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
+static __always_inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
unsigned int pid,
unsigned int ric, unsigned int prs)
{
@@ -150,8 +150,8 @@ static __always_inline void __tlbie_lpid(unsigned long lpid, unsigned long ric)
trace_tlbie(lpid, 0, rb, rs, ric, prs, r);
}
-static inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
- unsigned long ric)
+static __always_inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
+ unsigned long ric)
{
unsigned long rb,rs,prs,r;
@@ -167,8 +167,8 @@ static inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
}
-static inline void __tlbiel_va(unsigned long va, unsigned long pid,
- unsigned long ap, unsigned long ric)
+static __always_inline void __tlbiel_va(unsigned long va, unsigned long pid,
+ unsigned long ap, unsigned long ric)
{
unsigned long rb,rs,prs,r;
@@ -183,8 +183,8 @@ static inline void __tlbiel_va(unsigned long va, unsigned long pid,
trace_tlbie(0, 1, rb, rs, ric, prs, r);
}
-static inline void __tlbie_va(unsigned long va, unsigned long pid,
- unsigned long ap, unsigned long ric)
+static __always_inline void __tlbie_va(unsigned long va, unsigned long pid,
+ unsigned long ap, unsigned long ric)
{
unsigned long rb,rs,prs,r;
@@ -199,8 +199,8 @@ static inline void __tlbie_va(unsigned long va, unsigned long pid,
trace_tlbie(0, 0, rb, rs, ric, prs, r);
}
-static inline void __tlbie_lpid_va(unsigned long va, unsigned long lpid,
- unsigned long ap, unsigned long ric)
+static __always_inline void __tlbie_lpid_va(unsigned long va, unsigned long lpid,
+ unsigned long ap, unsigned long ric)
{
unsigned long rb,rs,prs,r;
@@ -239,7 +239,7 @@ static inline void fixup_tlbie_lpid(unsigned long lpid)
/*
* We use 128 set in radix mode and 256 set in hpt mode.
*/
-static inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
+static __always_inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
{
int set;
@@ -341,7 +341,7 @@ static inline void _tlbie_lpid(unsigned long lpid, unsigned long ric)
asm volatile("eieio; tlbsync; ptesync": : :"memory");
}
-static inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
+static __always_inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
{
int set;
@@ -381,8 +381,8 @@ static inline void __tlbiel_va_range(unsigned long start, unsigned long end,
__tlbiel_va(addr, pid, ap, RIC_FLUSH_TLB);
}
-static inline void _tlbiel_va(unsigned long va, unsigned long pid,
- unsigned long psize, unsigned long ric)
+static __always_inline void _tlbiel_va(unsigned long va, unsigned long pid,
+ unsigned long psize, unsigned long ric)
{
unsigned long ap = mmu_get_ap(psize);
@@ -413,8 +413,8 @@ static inline void __tlbie_va_range(unsigned long start, unsigned long end,
__tlbie_va(addr, pid, ap, RIC_FLUSH_TLB);
}
-static inline void _tlbie_va(unsigned long va, unsigned long pid,
- unsigned long psize, unsigned long ric)
+static __always_inline void _tlbie_va(unsigned long va, unsigned long pid,
+ unsigned long psize, unsigned long ric)
{
unsigned long ap = mmu_get_ap(psize);
@@ -424,7 +424,7 @@ static inline void _tlbie_va(unsigned long va, unsigned long pid,
asm volatile("eieio; tlbsync; ptesync": : :"memory");
}
-static inline void _tlbie_lpid_va(unsigned long va, unsigned long lpid,
+static __always_inline void _tlbie_lpid_va(unsigned long va, unsigned long lpid,
unsigned long psize, unsigned long ric)
{
unsigned long ap = mmu_get_ap(psize);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-21 13:18 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-ia64, linux-sh, linux-kernel, dhowells, linux-kselftest,
sparclinux, shuah, linux-arch, linux-s390, miklos, x86, torvalds,
linux-mips, linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro,
tglx, ldv, linux-arm-kernel, linux-parisc, linux-api, oleg,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <87h89o9cng.fsf@oldenburg2.str.redhat.com>
On Tue, May 21, 2019 at 03:10:11PM +0200, Florian Weimer wrote:
> * Christian Brauner:
>
> >> Solaris has an fdwalk function:
> >>
> >> <https://docs.oracle.com/cd/E88353_01/html/E37843/closefrom-3c.html>
> >>
> >> So a different way to implement this would expose a nextfd system call
> >
> > Meh. If nextfd() then I would like it to be able to:
> > - get the nextfd(fd) >= fd
> > - get highest open fd e.g. nextfd(-1)
>
> The highest open descriptor isn't istering for fdwalk because nextfd
> would just fail.
>
> > But then I wonder if nextfd() needs to be a syscall and isn't just
> > either:
> > fcntl(fd, F_GET_NEXT)?
> > or
> > prctl(PR_GET_NEXT)?
>
> I think the fcntl route is a bit iffy because you might need it to get
> the *first* valid descriptor.
Oh, how would that be difficult? Maybe I'm missing context.
Couldn't you just do
fcntl(0, F_GET_NEXT)
>
> >> to userspace, so that we can use that to implement both fdwalk and
> >> closefrom. But maybe fdwalk is just too obscure, given the existence of
> >> /proc.
> >
> > Yeah we probably don't need fdwalk.
>
> Agreed. Just wanted to bring it up for completeness. I certainly don't
> want to derail the implementation of close_range.
No, that's perfectly fine. I mean, you clearly need this and are one of
the major stakeholders. For example, Rust (probably also Python) will
call down into libc and not use the syscall directly. They kinda do this
with getfdtable<sm> rn already.
So what you say makes sense for libc has some relevance for the other
tools as well.
Christian
^ permalink raw reply
* Re: [PATCH 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-21 13:23 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-ia64, linux-sh, linux-kernel, dhowells, linux-kselftest,
sparclinux, shuah, linux-arch, linux-s390, miklos, x86, torvalds,
linux-mips, linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro,
tglx, ldv, linux-arm-kernel, linux-parisc, linux-api, oleg,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <87h89o9cng.fsf@oldenburg2.str.redhat.com>
On Tue, May 21, 2019 at 03:10:11PM +0200, Florian Weimer wrote:
> * Christian Brauner:
>
> >> Solaris has an fdwalk function:
> >>
> >> <https://docs.oracle.com/cd/E88353_01/html/E37843/closefrom-3c.html>
> >>
> >> So a different way to implement this would expose a nextfd system call
> >
> > Meh. If nextfd() then I would like it to be able to:
> > - get the nextfd(fd) >= fd
> > - get highest open fd e.g. nextfd(-1)
>
> The highest open descriptor isn't istering for fdwalk because nextfd
> would just fail.
Sure. I was thinking about other usecases. For example, sometimes in
userspace you want to do the following:
save_fd = dup(fd, <well-known-number-at-the-end-of-the-range);
close_range(3, (save_fd - 1));
Which brings me to another point. So even if we don't do close_range() I
would like libc to maybe give us something like close_range() for such
scenarios.
>
> > But then I wonder if nextfd() needs to be a syscall and isn't just
> > either:
> > fcntl(fd, F_GET_NEXT)?
> > or
> > prctl(PR_GET_NEXT)?
>
> I think the fcntl route is a bit iffy because you might need it to get
> the *first* valid descriptor.
>
> >> to userspace, so that we can use that to implement both fdwalk and
> >> closefrom. But maybe fdwalk is just too obscure, given the existence of
> >> /proc.
> >
> > Yeah we probably don't need fdwalk.
>
> Agreed. Just wanted to bring it up for completeness. I certainly don't
> want to derail the implementation of close_range.
>
> Thanks,
> Florian
^ permalink raw reply
* [PATCH v1 00/15] Fixing selftests failure on Talitos driver
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
Several test failures have popped up following recent changes to crypto
selftests.
This series fixes (most of) them.
The last three patches are trivial cleanups.
Christophe Leroy (15):
crypto: talitos - fix skcipher failure due to wrong output IV
crypto: talitos - rename alternative AEAD algos.
crypto: talitos - reduce max key size for SEC1
crypto: talitos - check AES key size
crypto: talitos - fix CTR alg blocksize
crypto: talitos - check data blocksize in ablkcipher.
crypto: talitos - fix ECB algs ivsize
crypto: talitos - Do not modify req->cryptlen on decryption.
crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.
crypto: talitos - properly handle split ICV.
crypto: talitos - Align SEC1 accesses to 32 bits boundaries.
crypto: talitos - fix AEAD processing.
Revert "crypto: talitos - export the talitos_submit function"
crypto: talitos - use IS_ENABLED() in has_ftr_sec1()
crypto: talitos - use SPDX-License-Identifier
drivers/crypto/talitos.c | 281 ++++++++++++++++++++++-------------------------
drivers/crypto/talitos.h | 45 ++------
2 files changed, 139 insertions(+), 187 deletions(-)
--
2.13.3
^ permalink raw reply
* [PATCH v1 03/15] crypto: talitos - reduce max key size for SEC1
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
SEC1 doesn't support SHA384/512, so it doesn't require
longer keys.
This patch reduces the max key size when the driver
is built for SEC1 only.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 03d2c5114c95 ("crypto: talitos - Extend max key length for SHA384/512-HMAC and AEAD")
---
drivers/crypto/talitos.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6f8bc6467706..6312f8d501b1 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -837,7 +837,11 @@ static void talitos_unregister_rng(struct device *dev)
* HMAC_SNOOP_NO_AFEA (HSNA) instead of type IPSEC_ESP
*/
#define TALITOS_CRA_PRIORITY_AEAD_HSNA (TALITOS_CRA_PRIORITY - 1)
+#ifdef CONFIG_CRYPTO_DEV_TALITOS_SEC2
#define TALITOS_MAX_KEY_SIZE (AES_MAX_KEY_SIZE + SHA512_BLOCK_SIZE)
+#else
+#define TALITOS_MAX_KEY_SIZE (AES_MAX_KEY_SIZE + SHA256_BLOCK_SIZE)
+#endif
#define TALITOS_MAX_IV_LENGTH 16 /* max of AES_BLOCK_SIZE, DES3_EDE_BLOCK_SIZE */
struct talitos_ctx {
--
2.13.3
^ permalink raw reply related
* [PATCH v1 01/15] crypto: talitos - fix skcipher failure due to wrong output IV
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
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] 00000000: 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] 00000000: 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] 00000000: 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 <christophe.leroy@c-s.fr>
Fixes: 4de9d0b547b9 ("crypto: talitos - Add ablkcipher algorithms")
Cc: stable@vger.kernel.org
Reviewed-by: Horia Geanta <horia.geanta@nxp.com>
---
drivers/crypto/talitos.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 1d429fc073d1..f443cbe7da80 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1637,11 +1637,15 @@ static void ablkcipher_done(struct device *dev,
int err)
{
struct ablkcipher_request *areq = context;
+ struct crypto_ablkcipher *cipher = crypto_ablkcipher_reqtfm(areq);
+ struct talitos_ctx *ctx = crypto_ablkcipher_ctx(cipher);
+ unsigned int ivsize = crypto_ablkcipher_ivsize(cipher);
struct talitos_edesc *edesc;
edesc = container_of(desc, struct talitos_edesc, desc);
common_nonsnoop_unmap(dev, edesc, areq);
+ memcpy(areq->info, ctx->iv, ivsize);
kfree(edesc);
--
2.13.3
^ permalink raw reply related
* [PATCH v1 02/15] crypto: talitos - rename alternative AEAD algos.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
The talitos driver has two ways to perform AEAD depending on the
HW capability. Some HW support both. It is needed to give them
different names to distingish which one it is for instance when
a test fails.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 7405c8d7ff97 ("crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU")
Cc: stable@vger.kernel.org
---
drivers/crypto/talitos.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index f443cbe7da80..6f8bc6467706 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2356,7 +2356,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha1),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha1-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2401,7 +2401,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha1),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha1-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2444,7 +2444,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha224),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha224-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2489,7 +2489,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha224),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha224-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2532,7 +2532,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha256),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha256-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2577,7 +2577,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha256),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha256-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2706,7 +2706,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(md5),cbc(aes))",
.cra_driver_name = "authenc-hmac-md5-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2749,7 +2749,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(md5),cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-md5-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
--
2.13.3
^ permalink raw reply related
* [PATCH v1 04/15] crypto: talitos - check AES key size
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
Although the HW accepts any size and silently truncates
it to the correct length, the extra tests expects EINVAL
to be returned when the key size is not valid.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 4de9d0b547b9 ("crypto: talitos - Add ablkcipher algorithms")
---
drivers/crypto/talitos.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6312f8d501b1..95f71e18bf55 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1622,6 +1622,18 @@ static int ablkcipher_des3_setkey(struct crypto_ablkcipher *cipher,
return ablkcipher_setkey(cipher, key, keylen);
}
+static int ablkcipher_aes_setkey(struct crypto_ablkcipher *cipher,
+ const u8 *key, unsigned int keylen)
+{
+ if (keylen == AES_KEYSIZE_128 || keylen == AES_KEYSIZE_192 ||
+ keylen == AES_KEYSIZE_256)
+ return ablkcipher_setkey(cipher, key, keylen);
+
+ crypto_ablkcipher_set_flags(cipher, CRYPTO_TFM_RES_BAD_KEY_LEN);
+
+ return -EINVAL;
+}
+
static void common_nonsnoop_unmap(struct device *dev,
struct talitos_edesc *edesc,
struct ablkcipher_request *areq)
@@ -2782,6 +2794,7 @@ static struct talitos_alg_template driver_algs[] = {
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
.ivsize = AES_BLOCK_SIZE,
+ .setkey = ablkcipher_aes_setkey,
}
},
.desc_hdr_template = DESC_HDR_TYPE_COMMON_NONSNOOP_NO_AFEU |
@@ -2798,6 +2811,7 @@ static struct talitos_alg_template driver_algs[] = {
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
.ivsize = AES_BLOCK_SIZE,
+ .setkey = ablkcipher_aes_setkey,
}
},
.desc_hdr_template = DESC_HDR_TYPE_COMMON_NONSNOOP_NO_AFEU |
@@ -2815,6 +2829,7 @@ static struct talitos_alg_template driver_algs[] = {
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
.ivsize = AES_BLOCK_SIZE,
+ .setkey = ablkcipher_aes_setkey,
}
},
.desc_hdr_template = DESC_HDR_TYPE_AESU_CTR_NONSNOOP |
--
2.13.3
^ permalink raw reply related
* [PATCH v1 05/15] crypto: talitos - fix CTR alg blocksize
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
CTR has a blocksize of 1.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
---
drivers/crypto/talitos.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 95f71e18bf55..8b9a529f1b66 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2822,7 +2822,7 @@ static struct talitos_alg_template driver_algs[] = {
.alg.crypto = {
.cra_name = "ctr(aes)",
.cra_driver_name = "ctr-aes-talitos",
- .cra_blocksize = AES_BLOCK_SIZE,
+ .cra_blocksize = 1,
.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER |
CRYPTO_ALG_ASYNC,
.cra_ablkcipher = {
--
2.13.3
^ permalink raw reply related
* [PATCH v1 07/15] crypto: talitos - fix ECB algs ivsize
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
ECB's ivsize must be 0.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
---
drivers/crypto/talitos.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 1e5410f92166..6f6f34754ad8 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2809,7 +2809,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
.setkey = ablkcipher_aes_setkey,
}
},
@@ -2862,7 +2861,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = DES_KEY_SIZE,
.max_keysize = DES_KEY_SIZE,
- .ivsize = DES_BLOCK_SIZE,
.setkey = ablkcipher_des_setkey,
}
},
@@ -2897,7 +2895,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = DES3_EDE_KEY_SIZE,
.max_keysize = DES3_EDE_KEY_SIZE,
- .ivsize = DES3_EDE_BLOCK_SIZE,
.setkey = ablkcipher_des3_setkey,
}
},
--
2.13.3
^ permalink raw reply related
* [PATCH v1 08/15] crypto: talitos - Do not modify req->cryptlen on decryption.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
For decrypt, req->cryptlen includes the size of the authentication
part while all functions of the driver expect cryptlen to be
the size of the encrypted data.
As it is not expected to change req->cryptlen, this patch
implements local calculation of cryptlen.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 9c4a79653b35 ("crypto: talitos - Freescale integrated security engine (SEC) driver")
---
drivers/crypto/talitos.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6f6f34754ad8..a15aa6d6ec33 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1025,11 +1025,13 @@ static void talitos_sg_unmap(struct device *dev,
static void ipsec_esp_unmap(struct device *dev,
struct talitos_edesc *edesc,
- struct aead_request *areq)
+ struct aead_request *areq, bool encrypt)
{
struct crypto_aead *aead = crypto_aead_reqtfm(areq);
struct talitos_ctx *ctx = crypto_aead_ctx(aead);
unsigned int ivsize = crypto_aead_ivsize(aead);
+ unsigned int authsize = crypto_aead_authsize(aead);
+ unsigned int cryptlen = areq->cryptlen - (encrypt ? 0 : authsize);
bool is_ipsec_esp = edesc->desc.hdr & DESC_HDR_TYPE_IPSEC_ESP;
struct talitos_ptr *civ_ptr = &edesc->desc.ptr[is_ipsec_esp ? 2 : 3];
@@ -1038,7 +1040,7 @@ static void ipsec_esp_unmap(struct device *dev,
DMA_FROM_DEVICE);
unmap_single_talitos_ptr(dev, civ_ptr, DMA_TO_DEVICE);
- talitos_sg_unmap(dev, edesc, areq->src, areq->dst, areq->cryptlen,
+ talitos_sg_unmap(dev, edesc, areq->src, areq->dst, cryptlen,
areq->assoclen);
if (edesc->dma_len)
@@ -1049,7 +1051,7 @@ static void ipsec_esp_unmap(struct device *dev,
unsigned int dst_nents = edesc->dst_nents ? : 1;
sg_pcopy_to_buffer(areq->dst, dst_nents, ctx->iv, ivsize,
- areq->assoclen + areq->cryptlen - ivsize);
+ areq->assoclen + cryptlen - ivsize);
}
}
@@ -1072,7 +1074,7 @@ static void ipsec_esp_encrypt_done(struct device *dev,
edesc = container_of(desc, struct talitos_edesc, desc);
- ipsec_esp_unmap(dev, edesc, areq);
+ ipsec_esp_unmap(dev, edesc, areq, true);
/* copy the generated ICV to dst */
if (edesc->icv_ool) {
@@ -1108,7 +1110,7 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
edesc = container_of(desc, struct talitos_edesc, desc);
- ipsec_esp_unmap(dev, edesc, req);
+ ipsec_esp_unmap(dev, edesc, req, false);
if (!err) {
/* auth check */
@@ -1145,7 +1147,7 @@ static void ipsec_esp_decrypt_hwauth_done(struct device *dev,
edesc = container_of(desc, struct talitos_edesc, desc);
- ipsec_esp_unmap(dev, edesc, req);
+ ipsec_esp_unmap(dev, edesc, req, false);
/* check ICV auth status */
if (!err && ((desc->hdr_lo & DESC_HDR_LO_ICCR1_MASK) !=
@@ -1248,6 +1250,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
* fill in and submit ipsec_esp descriptor
*/
static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
+ bool encrypt,
void (*callback)(struct device *dev,
struct talitos_desc *desc,
void *context, int error))
@@ -1257,7 +1260,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
struct talitos_ctx *ctx = crypto_aead_ctx(aead);
struct device *dev = ctx->dev;
struct talitos_desc *desc = &edesc->desc;
- unsigned int cryptlen = areq->cryptlen;
+ unsigned int cryptlen = areq->cryptlen - (encrypt ? 0 : authsize);
unsigned int ivsize = crypto_aead_ivsize(aead);
int tbl_off = 0;
int sg_count, ret;
@@ -1384,7 +1387,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
- ipsec_esp_unmap(dev, edesc, areq);
+ ipsec_esp_unmap(dev, edesc, areq, encrypt);
kfree(edesc);
}
return ret;
@@ -1502,9 +1505,10 @@ static struct talitos_edesc *aead_edesc_alloc(struct aead_request *areq, u8 *iv,
unsigned int authsize = crypto_aead_authsize(authenc);
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
unsigned int ivsize = crypto_aead_ivsize(authenc);
+ unsigned int cryptlen = areq->cryptlen - (encrypt ? 0 : authsize);
return talitos_edesc_alloc(ctx->dev, areq->src, areq->dst,
- iv, areq->assoclen, areq->cryptlen,
+ iv, areq->assoclen, cryptlen,
authsize, ivsize, icv_stashing,
areq->base.flags, encrypt);
}
@@ -1523,7 +1527,7 @@ static int aead_encrypt(struct aead_request *req)
/* set encrypt */
edesc->desc.hdr = ctx->desc_hdr_template | DESC_HDR_MODE0_ENCRYPT;
- return ipsec_esp(edesc, req, ipsec_esp_encrypt_done);
+ return ipsec_esp(edesc, req, true, ipsec_esp_encrypt_done);
}
static int aead_decrypt(struct aead_request *req)
@@ -1536,8 +1540,6 @@ static int aead_decrypt(struct aead_request *req)
struct scatterlist *sg;
void *icvdata;
- req->cryptlen -= authsize;
-
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, req->iv, 1, false);
if (IS_ERR(edesc))
@@ -1554,7 +1556,8 @@ static int aead_decrypt(struct aead_request *req)
/* reset integrity check result bits */
- return ipsec_esp(edesc, req, ipsec_esp_decrypt_hwauth_done);
+ return ipsec_esp(edesc, req, false,
+ ipsec_esp_decrypt_hwauth_done);
}
/* Have to check the ICV with software */
@@ -1571,7 +1574,7 @@ static int aead_decrypt(struct aead_request *req)
memcpy(icvdata, (char *)sg_virt(sg) + sg->length - authsize, authsize);
- return ipsec_esp(edesc, req, ipsec_esp_decrypt_swauth_done);
+ return ipsec_esp(edesc, req, false, ipsec_esp_decrypt_swauth_done);
}
static int ablkcipher_setkey(struct crypto_ablkcipher *cipher,
--
2.13.3
^ permalink raw reply related
* [PATCH v1 09/15] crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
In that mode, hardware ICV verification is not supported.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 7405c8d7ff97 ("crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU")
---
drivers/crypto/talitos.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index a15aa6d6ec33..e35581d67315 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1545,7 +1545,8 @@ static int aead_decrypt(struct aead_request *req)
if (IS_ERR(edesc))
return PTR_ERR(edesc);
- if ((priv->features & TALITOS_FTR_HW_AUTH_CHECK) &&
+ if ((edesc->desc.hdr & DESC_HDR_TYPE_IPSEC_ESP) &&
+ (priv->features & TALITOS_FTR_HW_AUTH_CHECK) &&
((!edesc->src_nents && !edesc->dst_nents) ||
priv->features & TALITOS_FTR_SRC_LINK_TBL_LEN_INCLUDES_EXTENT)) {
--
2.13.3
^ permalink raw reply related
* [PATCH v1 10/15] crypto: talitos - properly handle split ICV.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
The driver assumes that the ICV is as a single piece in the last
element of the scatterlist. This assumption is wrong.
This patch ensures that the ICV is properly handled regardless of
the scatterlist layout.
Fixes: 9c4a79653b35 ("crypto: talitos - Freescale integrated security engine (SEC) driver")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index e35581d67315..7c8a3a717b91 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1069,7 +1069,6 @@ static void ipsec_esp_encrypt_done(struct device *dev,
unsigned int authsize = crypto_aead_authsize(authenc);
unsigned int ivsize = crypto_aead_ivsize(authenc);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
void *icvdata;
edesc = container_of(desc, struct talitos_edesc, desc);
@@ -1083,9 +1082,8 @@ static void ipsec_esp_encrypt_done(struct device *dev,
else
icvdata = &edesc->link_tbl[edesc->src_nents +
edesc->dst_nents + 2];
- sg = sg_last(areq->dst, edesc->dst_nents);
- memcpy((char *)sg_virt(sg) + sg->length - authsize,
- icvdata, authsize);
+ sg_pcopy_from_buffer(areq->dst, edesc->dst_nents ? : 1, icvdata,
+ authsize, areq->assoclen + areq->cryptlen);
}
dma_unmap_single(dev, edesc->iv_dma, ivsize, DMA_TO_DEVICE);
@@ -1103,7 +1101,6 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
struct crypto_aead *authenc = crypto_aead_reqtfm(req);
unsigned int authsize = crypto_aead_authsize(authenc);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
char *oicv, *icv;
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
@@ -1113,9 +1110,18 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
ipsec_esp_unmap(dev, edesc, req, false);
if (!err) {
+ char icvdata[SHA512_DIGEST_SIZE];
+ int nents = edesc->dst_nents ? : 1;
+ unsigned int len = req->assoclen + req->cryptlen;
+
/* auth check */
- sg = sg_last(req->dst, edesc->dst_nents ? : 1);
- icv = (char *)sg_virt(sg) + sg->length - authsize;
+ if (nents > 1) {
+ sg_pcopy_to_buffer(req->dst, nents, icvdata, authsize,
+ len - authsize);
+ icv = icvdata;
+ } else {
+ icv = (char *)sg_virt(req->dst) + len - authsize;
+ }
if (edesc->dma_len) {
if (is_sec1)
@@ -1537,7 +1543,6 @@ static int aead_decrypt(struct aead_request *req)
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
struct talitos_private *priv = dev_get_drvdata(ctx->dev);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
void *icvdata;
/* allocate extended descriptor */
@@ -1571,9 +1576,8 @@ static int aead_decrypt(struct aead_request *req)
else
icvdata = &edesc->link_tbl[0];
- sg = sg_last(req->src, edesc->src_nents ? : 1);
-
- memcpy(icvdata, (char *)sg_virt(sg) + sg->length - authsize, authsize);
+ sg_pcopy_to_buffer(req->src, edesc->src_nents ? : 1, icvdata, authsize,
+ req->assoclen + req->cryptlen - authsize);
return ipsec_esp(edesc, req, false, ipsec_esp_decrypt_swauth_done);
}
--
2.13.3
^ permalink raw reply related
* [PATCH v1 06/15] crypto: talitos - check data blocksize in ablkcipher.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
When data size is not a multiple of the alg's block size,
the SEC generates an error interrupt and dumps the registers.
And for NULL size, the SEC does just nothing and the interrupt
is awaited forever.
This patch ensures the data size is correct before submitting
the request to the SEC engine.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 4de9d0b547b9 ("crypto: talitos - Add ablkcipher algorithms")
---
drivers/crypto/talitos.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 8b9a529f1b66..1e5410f92166 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1756,6 +1756,14 @@ static int ablkcipher_encrypt(struct ablkcipher_request *areq)
struct crypto_ablkcipher *cipher = crypto_ablkcipher_reqtfm(areq);
struct talitos_ctx *ctx = crypto_ablkcipher_ctx(cipher);
struct talitos_edesc *edesc;
+ unsigned int blocksize =
+ crypto_tfm_alg_blocksize(crypto_ablkcipher_tfm(cipher));
+
+ if (!areq->nbytes)
+ return 0;
+
+ if (areq->nbytes % blocksize)
+ return -EINVAL;
/* allocate extended descriptor */
edesc = ablkcipher_edesc_alloc(areq, true);
@@ -1773,6 +1781,14 @@ static int ablkcipher_decrypt(struct ablkcipher_request *areq)
struct crypto_ablkcipher *cipher = crypto_ablkcipher_reqtfm(areq);
struct talitos_ctx *ctx = crypto_ablkcipher_ctx(cipher);
struct talitos_edesc *edesc;
+ unsigned int blocksize =
+ crypto_tfm_alg_blocksize(crypto_ablkcipher_tfm(cipher));
+
+ if (!areq->nbytes)
+ return 0;
+
+ if (areq->nbytes % blocksize)
+ return -EINVAL;
/* allocate extended descriptor */
edesc = ablkcipher_edesc_alloc(areq, false);
--
2.13.3
^ permalink raw reply related
* [PATCH v1 11/15] crypto: talitos - Align SEC1 accesses to 32 bits boundaries.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
The MPC885 reference manual states:
SEC Lite-initiated 8xx writes can occur only on 32-bit-word boundaries, but
reads can occur on any byte boundary. Writing back a header read from a
non-32-bit-word boundary will yield unpredictable results.
In order to ensure that, cra_alignmask is set to 3 for SEC1.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 9c4a79653b35 ("crypto: talitos - Freescale integrated security engine (SEC) driver")
---
drivers/crypto/talitos.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 7c8a3a717b91..750b0159e654 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -3327,7 +3327,10 @@ static struct talitos_crypto_alg *talitos_alg_alloc(struct device *dev,
alg->cra_priority = t_alg->algt.priority;
else
alg->cra_priority = TALITOS_CRA_PRIORITY;
- alg->cra_alignmask = 0;
+ if (has_ftr_sec1(priv))
+ alg->cra_alignmask = 3;
+ else
+ alg->cra_alignmask = 0;
alg->cra_ctxsize = sizeof(struct talitos_ctx);
alg->cra_flags |= CRYPTO_ALG_KERN_DRIVER_ONLY;
--
2.13.3
^ permalink raw reply related
* [PATCH v1 12/15] crypto: talitos - fix AEAD processing.
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
This driver is working well in 'simple cases', but as soon as
more exotic SG lists are provided (dst different from src,
auth part not in a single SG fragment, ...) there are
wrong results, overruns, etc ...
This patch cleans up the AEAD processing by:
- Simplifying the location of 'out of line' ICV
- Never using 'out of line' ICV on encryp
- Always using 'out of line' ICV on decrypt
- Forcing the generation of a SG table on decrypt
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: aeb4c132f33d ("crypto: talitos - Convert to new AEAD interface")
---
drivers/crypto/talitos.c | 158 ++++++++++++++++-------------------------------
drivers/crypto/talitos.h | 2 +-
2 files changed, 55 insertions(+), 105 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 750b0159e654..12917fd54bcb 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1040,8 +1040,8 @@ static void ipsec_esp_unmap(struct device *dev,
DMA_FROM_DEVICE);
unmap_single_talitos_ptr(dev, civ_ptr, DMA_TO_DEVICE);
- talitos_sg_unmap(dev, edesc, areq->src, areq->dst, cryptlen,
- areq->assoclen);
+ talitos_sg_unmap(dev, edesc, areq->src, areq->dst,
+ cryptlen + authsize, areq->assoclen);
if (edesc->dma_len)
dma_unmap_single(dev, edesc->dma_link_tbl, edesc->dma_len,
@@ -1062,30 +1062,15 @@ static void ipsec_esp_encrypt_done(struct device *dev,
struct talitos_desc *desc, void *context,
int err)
{
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
struct aead_request *areq = context;
struct crypto_aead *authenc = crypto_aead_reqtfm(areq);
- unsigned int authsize = crypto_aead_authsize(authenc);
unsigned int ivsize = crypto_aead_ivsize(authenc);
struct talitos_edesc *edesc;
- void *icvdata;
edesc = container_of(desc, struct talitos_edesc, desc);
ipsec_esp_unmap(dev, edesc, areq, true);
- /* copy the generated ICV to dst */
- if (edesc->icv_ool) {
- if (is_sec1)
- icvdata = edesc->buf + areq->assoclen + areq->cryptlen;
- else
- icvdata = &edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- sg_pcopy_from_buffer(areq->dst, edesc->dst_nents ? : 1, icvdata,
- authsize, areq->assoclen + areq->cryptlen);
- }
-
dma_unmap_single(dev, edesc->iv_dma, ivsize, DMA_TO_DEVICE);
kfree(edesc);
@@ -1102,39 +1087,15 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
unsigned int authsize = crypto_aead_authsize(authenc);
struct talitos_edesc *edesc;
char *oicv, *icv;
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
edesc = container_of(desc, struct talitos_edesc, desc);
ipsec_esp_unmap(dev, edesc, req, false);
if (!err) {
- char icvdata[SHA512_DIGEST_SIZE];
- int nents = edesc->dst_nents ? : 1;
- unsigned int len = req->assoclen + req->cryptlen;
-
/* auth check */
- if (nents > 1) {
- sg_pcopy_to_buffer(req->dst, nents, icvdata, authsize,
- len - authsize);
- icv = icvdata;
- } else {
- icv = (char *)sg_virt(req->dst) + len - authsize;
- }
-
- if (edesc->dma_len) {
- if (is_sec1)
- oicv = (char *)&edesc->dma_link_tbl +
- req->assoclen + req->cryptlen;
- else
- oicv = (char *)
- &edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- if (edesc->icv_ool)
- icv = oicv + authsize;
- } else
- oicv = (char *)&edesc->link_tbl[0];
+ oicv = edesc->buf + edesc->dma_len;
+ icv = oicv - authsize;
err = crypto_memneq(oicv, icv, authsize) ? -EBADMSG : 0;
}
@@ -1170,11 +1131,12 @@ static void ipsec_esp_decrypt_hwauth_done(struct device *dev,
* stop at cryptlen bytes
*/
static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
- unsigned int offset, int cryptlen,
+ unsigned int offset, int datalen, int elen,
struct talitos_ptr *link_tbl_ptr)
{
- int n_sg = sg_count;
+ int n_sg = elen ? sg_count + 1 : sg_count;
int count = 0;
+ int cryptlen = datalen + elen;
while (cryptlen && sg && n_sg--) {
unsigned int len = sg_dma_len(sg);
@@ -1189,11 +1151,20 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
if (len > cryptlen)
len = cryptlen;
+ if (datalen > 0 && len > datalen) {
+ to_talitos_ptr(link_tbl_ptr + count,
+ sg_dma_address(sg) + offset, datalen, 0);
+ to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
+ count++;
+ len -= datalen;
+ offset += datalen;
+ }
to_talitos_ptr(link_tbl_ptr + count,
sg_dma_address(sg) + offset, len, 0);
to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
count++;
cryptlen -= len;
+ datalen -= len;
offset = 0;
next:
@@ -1203,7 +1174,7 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
/* tag end of link table */
if (count > 0)
to_talitos_ptr_ext_set(link_tbl_ptr + count - 1,
- DESC_PTR_LNKTBL_RETURN, 0);
+ DESC_PTR_LNKTBL_RET, 0);
return count;
}
@@ -1211,7 +1182,8 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
unsigned int len, struct talitos_edesc *edesc,
struct talitos_ptr *ptr, int sg_count,
- unsigned int offset, int tbl_off, int elen)
+ unsigned int offset, int tbl_off, int elen,
+ bool force)
{
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
@@ -1221,7 +1193,7 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
return 1;
}
to_talitos_ptr_ext_set(ptr, elen, is_sec1);
- if (sg_count == 1) {
+ if (sg_count == 1 && !force) {
to_talitos_ptr(ptr, sg_dma_address(src) + offset, len, is_sec1);
return sg_count;
}
@@ -1229,9 +1201,9 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, len, is_sec1);
return sg_count;
}
- sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len + elen,
+ sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len, elen,
&edesc->link_tbl[tbl_off]);
- if (sg_count == 1) {
+ if (sg_count == 1 && !force) {
/* Only one segment now, so no link tbl needed*/
copy_talitos_ptr(ptr, &edesc->link_tbl[tbl_off], is_sec1);
return sg_count;
@@ -1249,7 +1221,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
unsigned int offset, int tbl_off)
{
return talitos_sg_map_ext(dev, src, len, edesc, ptr, sg_count, offset,
- tbl_off, 0);
+ tbl_off, 0, false);
}
/*
@@ -1277,6 +1249,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
bool is_ipsec_esp = desc->hdr & DESC_HDR_TYPE_IPSEC_ESP;
struct talitos_ptr *civ_ptr = &desc->ptr[is_ipsec_esp ? 2 : 3];
struct talitos_ptr *ckey_ptr = &desc->ptr[is_ipsec_esp ? 3 : 2];
+ dma_addr_t dma_icv = edesc->dma_link_tbl + edesc->dma_len - authsize;
/* hmac key */
to_talitos_ptr(&desc->ptr[0], ctx->dma_key, ctx->authkeylen, is_sec1);
@@ -1316,7 +1289,8 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
elen = authsize;
ret = talitos_sg_map_ext(dev, areq->src, cryptlen, edesc, &desc->ptr[4],
- sg_count, areq->assoclen, tbl_off, elen);
+ sg_count, areq->assoclen, tbl_off, elen,
+ false);
if (ret > 1) {
tbl_off += ret;
@@ -1330,55 +1304,35 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
dma_map_sg(dev, areq->dst, sg_count, DMA_FROM_DEVICE);
}
- ret = talitos_sg_map(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
- sg_count, areq->assoclen, tbl_off);
-
- if (is_ipsec_esp)
- to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
+ if (is_ipsec_esp && encrypt)
+ elen = authsize;
+ else
+ elen = 0;
+ ret = talitos_sg_map_ext(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
+ sg_count, areq->assoclen, tbl_off, elen,
+ is_ipsec_esp && !encrypt);
+ tbl_off += ret;
/* ICV data */
- if (ret > 1) {
- tbl_off += ret;
- edesc->icv_ool = true;
- sync_needed = true;
+ edesc->icv_ool = !encrypt;
- if (is_ipsec_esp) {
- struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
- int offset = (edesc->src_nents + edesc->dst_nents + 2) *
- sizeof(struct talitos_ptr) + authsize;
+ if (!encrypt && is_ipsec_esp) {
+ struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
- /* Add an entry to the link table for ICV data */
- to_talitos_ptr_ext_set(tbl_ptr - 1, 0, is_sec1);
- to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN,
- is_sec1);
+ /* Add an entry to the link table for ICV data */
+ to_talitos_ptr_ext_set(tbl_ptr - 1, 0, is_sec1);
+ to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RET, is_sec1);
- /* icv data follows link tables */
- to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
- authsize, is_sec1);
- } else {
- dma_addr_t addr = edesc->dma_link_tbl;
-
- if (is_sec1)
- addr += areq->assoclen + cryptlen;
- else
- addr += sizeof(struct talitos_ptr) * tbl_off;
-
- to_talitos_ptr(&desc->ptr[6], addr, authsize, is_sec1);
- }
+ /* icv data follows link tables */
+ to_talitos_ptr(tbl_ptr, dma_icv, authsize, is_sec1);
+ to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
+ sync_needed = true;
+ } else if (!encrypt) {
+ to_talitos_ptr(&desc->ptr[6], dma_icv, authsize, is_sec1);
+ sync_needed = true;
} else if (!is_ipsec_esp) {
- ret = talitos_sg_map(dev, areq->dst, authsize, edesc,
- &desc->ptr[6], sg_count, areq->assoclen +
- cryptlen,
- tbl_off);
- if (ret > 1) {
- tbl_off += ret;
- edesc->icv_ool = true;
- sync_needed = true;
- } else {
- edesc->icv_ool = false;
- }
- } else {
- edesc->icv_ool = false;
+ talitos_sg_map(dev, areq->dst, authsize, edesc, &desc->ptr[6],
+ sg_count, areq->assoclen + cryptlen, tbl_off);
}
/* iv out */
@@ -1461,18 +1415,18 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
* and space for two sets of ICVs (stashed and generated)
*/
alloc_len = sizeof(struct talitos_edesc);
- if (src_nents || dst_nents) {
+ if (src_nents || dst_nents || !encrypt) {
if (is_sec1)
dma_len = (src_nents ? src_len : 0) +
- (dst_nents ? dst_len : 0);
+ (dst_nents ? dst_len : 0) + authsize;
else
dma_len = (src_nents + dst_nents + 2) *
- sizeof(struct talitos_ptr) + authsize * 2;
+ sizeof(struct talitos_ptr) + authsize;
alloc_len += dma_len;
} else {
dma_len = 0;
- alloc_len += icv_stashing ? authsize : 0;
}
+ alloc_len += icv_stashing ? authsize : 0;
/* if its a ahash, add space for a second desc next to the first one */
if (is_sec1 && !dst)
@@ -1570,11 +1524,7 @@ static int aead_decrypt(struct aead_request *req)
edesc->desc.hdr = ctx->desc_hdr_template | DESC_HDR_DIR_INBOUND;
/* stash incoming ICV for later cmp with ICV generated by the h/w */
- if (edesc->dma_len)
- icvdata = (char *)&edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- else
- icvdata = &edesc->link_tbl[0];
+ icvdata = edesc->buf + edesc->dma_len;
sg_pcopy_to_buffer(req->src, edesc->src_nents ? : 1, icvdata, authsize,
req->assoclen + req->cryptlen - authsize);
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index a65a63e0d6c1..dbedd0956c8a 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -412,5 +412,5 @@ static inline bool has_ftr_sec1(struct talitos_private *priv)
/* link table extent field bits */
#define DESC_PTR_LNKTBL_JUMP 0x80
-#define DESC_PTR_LNKTBL_RETURN 0x02
+#define DESC_PTR_LNKTBL_RET 0x02
#define DESC_PTR_LNKTBL_NEXT 0x01
--
2.13.3
^ permalink raw reply related
* [PATCH v1 13/15] Revert "crypto: talitos - export the talitos_submit function"
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
There is no other file using talitos_submit in the kernel tree,
so it doesn't need to be exported nor made global.
This reverts commit 865d506155b117edc7e668ced373030ce7108ce9.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 865d506155b1 ("crypto: talitos - export the talitos_submit function")
---
drivers/crypto/talitos.c | 11 +++++------
drivers/crypto/talitos.h | 6 ------
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 12917fd54bcb..6fe900d8e5d8 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -278,11 +278,11 @@ static int init_device(struct device *dev)
* callback must check err and feedback in descriptor header
* for device processing status.
*/
-int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
- void (*callback)(struct device *dev,
- struct talitos_desc *desc,
- void *context, int error),
- void *context)
+static int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
+ void (*callback)(struct device *dev,
+ struct talitos_desc *desc,
+ void *context, int error),
+ void *context)
{
struct talitos_private *priv = dev_get_drvdata(dev);
struct talitos_request *request;
@@ -332,7 +332,6 @@ int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
return -EINPROGRESS;
}
-EXPORT_SYMBOL(talitos_submit);
/*
* process what was done, notify callback of error if not
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index dbedd0956c8a..95e97951b924 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -150,12 +150,6 @@ struct talitos_private {
bool rng_registered;
};
-extern int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
- void (*callback)(struct device *dev,
- struct talitos_desc *desc,
- void *context, int error),
- void *context);
-
/* .features flag */
#define TALITOS_FTR_SRC_LINK_TBL_LEN_INCLUDES_EXTENT 0x00000001
#define TALITOS_FTR_HW_AUTH_CHECK 0x00000002
--
2.13.3
^ permalink raw reply related
* [PATCH v1 14/15] crypto: talitos - use IS_ENABLED() in has_ftr_sec1()
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
This patch rewrites has_ftr_sec1() using IS_ENABLED()
instead of #ifdefs
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 95e97951b924..5699d46401e6 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -164,13 +164,11 @@ struct talitos_private {
*/
static inline bool has_ftr_sec1(struct talitos_private *priv)
{
-#if defined(CONFIG_CRYPTO_DEV_TALITOS1) && defined(CONFIG_CRYPTO_DEV_TALITOS2)
- return priv->features & TALITOS_FTR_SEC1 ? true : false;
-#elif defined(CONFIG_CRYPTO_DEV_TALITOS1)
- return true;
-#else
- return false;
-#endif
+ if (IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS1) &&
+ IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS2))
+ return priv->features & TALITOS_FTR_SEC1;
+
+ return IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS1);
}
/*
--
2.13.3
^ permalink raw reply related
* [PATCH v1 15/15] crypto: talitos - use SPDX-License-Identifier
From: Christophe Leroy @ 2019-05-21 13:34 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1558445259.git.christophe.leroy@c-s.fr>
This patch drops the license text and replaces it
with an SPDX-License-Identifier tag.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 15 +--------------
drivers/crypto/talitos.h | 25 +------------------------
2 files changed, 2 insertions(+), 38 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6fe900d8e5d8..32a7e747dc5f 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* talitos - Freescale Integrated Security Engine (SEC) device driver
*
@@ -9,20 +10,6 @@
* Crypto algorithm registration code copied from hifn driver:
* 2007+ Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
* All rights reserved.
- *
- * 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; either version 2 of the License, or
- * (at your option) any later version.
- *
- * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/kernel.h>
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 5699d46401e6..32ad4fc679ed 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -1,31 +1,8 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
/*
* Freescale SEC (talitos) device register and descriptor header defines
*
* Copyright (c) 2006-2011 Freescale Semiconductor, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. The name of the author may not be used to endorse or promote products
- * derived from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
*/
#define TALITOS_TIMEOUT 100000
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v2] powerpc/mm: mark more tlb functions as __always_inline
From: Christophe Leroy @ 2019-05-21 13:35 UTC (permalink / raw)
To: Masahiro Yamada, Michael Ellerman, linuxppc-dev
Cc: linux-kernel, Nicholas Piggin, Paul Mackerras, Aneesh Kumar K.V,
Suraj Jitindar Singh, Andrew Morton, David Gibson
In-Reply-To: <1558444404-12254-1-git-send-email-yamada.masahiro@socionext.com>
Le 21/05/2019 à 15:13, Masahiro Yamada a écrit :
> With CONFIG_OPTIMIZE_INLINING enabled, Laura Abbott reported error
> with gcc 9.1.1:
>
> arch/powerpc/mm/book3s64/radix_tlb.c: In function '_tlbiel_pid':
> arch/powerpc/mm/book3s64/radix_tlb.c:104:2: warning: asm operand 3 probably doesn't match constraints
> 104 | asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
> | ^~~
> arch/powerpc/mm/book3s64/radix_tlb.c:104:2: error: impossible constraint in 'asm'
>
> Fixing _tlbiel_pid() is enough to address the warning above, but I
> inlined more functions to fix all potential issues.
>
> To meet the "i" (immediate) constraint for the asm operands, functions
> propagating "ric" must be always inlined.
>
> Fixes: 9012d011660e ("compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING")
> Reported-by: Laura Abbott <labbott@redhat.com>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>
> Changes in v2:
> - Do not split lines
>
> arch/powerpc/mm/book3s64/hash_native.c | 2 +-
> arch/powerpc/mm/book3s64/radix_tlb.c | 32 ++++++++++++++++----------------
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
> index aaa28fd..c854151 100644
> --- a/arch/powerpc/mm/book3s64/hash_native.c
> +++ b/arch/powerpc/mm/book3s64/hash_native.c
> @@ -60,7 +60,7 @@ static inline void tlbiel_hash_set_isa206(unsigned int set, unsigned int is)
> * tlbiel instruction for hash, set invalidation
> * i.e., r=1 and is=01 or is=10 or is=11
> */
> -static inline void tlbiel_hash_set_isa300(unsigned int set, unsigned int is,
> +static __always_inline void tlbiel_hash_set_isa300(unsigned int set, unsigned int is,
> unsigned int pid,
> unsigned int ric, unsigned int prs)
> {
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 4d84136..4d3dc10 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -29,7 +29,7 @@
> * tlbiel instruction for radix, set invalidation
> * i.e., r=1 and is=01 or is=10 or is=11
> */
> -static inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
> +static __always_inline void tlbiel_radix_set_isa300(unsigned int set, unsigned int is,
> unsigned int pid,
> unsigned int ric, unsigned int prs)
> {
> @@ -150,8 +150,8 @@ static __always_inline void __tlbie_lpid(unsigned long lpid, unsigned long ric)
> trace_tlbie(lpid, 0, rb, rs, ric, prs, r);
> }
>
> -static inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
> - unsigned long ric)
> +static __always_inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
> + unsigned long ric)
> {
> unsigned long rb,rs,prs,r;
>
> @@ -167,8 +167,8 @@ static inline void __tlbiel_lpid_guest(unsigned long lpid, int set,
> }
>
>
> -static inline void __tlbiel_va(unsigned long va, unsigned long pid,
> - unsigned long ap, unsigned long ric)
> +static __always_inline void __tlbiel_va(unsigned long va, unsigned long pid,
> + unsigned long ap, unsigned long ric)
> {
> unsigned long rb,rs,prs,r;
>
> @@ -183,8 +183,8 @@ static inline void __tlbiel_va(unsigned long va, unsigned long pid,
> trace_tlbie(0, 1, rb, rs, ric, prs, r);
> }
>
> -static inline void __tlbie_va(unsigned long va, unsigned long pid,
> - unsigned long ap, unsigned long ric)
> +static __always_inline void __tlbie_va(unsigned long va, unsigned long pid,
> + unsigned long ap, unsigned long ric)
> {
> unsigned long rb,rs,prs,r;
>
> @@ -199,8 +199,8 @@ static inline void __tlbie_va(unsigned long va, unsigned long pid,
> trace_tlbie(0, 0, rb, rs, ric, prs, r);
> }
>
> -static inline void __tlbie_lpid_va(unsigned long va, unsigned long lpid,
> - unsigned long ap, unsigned long ric)
> +static __always_inline void __tlbie_lpid_va(unsigned long va, unsigned long lpid,
> + unsigned long ap, unsigned long ric)
> {
> unsigned long rb,rs,prs,r;
>
> @@ -239,7 +239,7 @@ static inline void fixup_tlbie_lpid(unsigned long lpid)
> /*
> * We use 128 set in radix mode and 256 set in hpt mode.
> */
> -static inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
> +static __always_inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
> {
> int set;
>
> @@ -341,7 +341,7 @@ static inline void _tlbie_lpid(unsigned long lpid, unsigned long ric)
> asm volatile("eieio; tlbsync; ptesync": : :"memory");
> }
>
> -static inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
> +static __always_inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
> {
> int set;
>
> @@ -381,8 +381,8 @@ static inline void __tlbiel_va_range(unsigned long start, unsigned long end,
> __tlbiel_va(addr, pid, ap, RIC_FLUSH_TLB);
> }
>
> -static inline void _tlbiel_va(unsigned long va, unsigned long pid,
> - unsigned long psize, unsigned long ric)
> +static __always_inline void _tlbiel_va(unsigned long va, unsigned long pid,
> + unsigned long psize, unsigned long ric)
> {
> unsigned long ap = mmu_get_ap(psize);
>
> @@ -413,8 +413,8 @@ static inline void __tlbie_va_range(unsigned long start, unsigned long end,
> __tlbie_va(addr, pid, ap, RIC_FLUSH_TLB);
> }
>
> -static inline void _tlbie_va(unsigned long va, unsigned long pid,
> - unsigned long psize, unsigned long ric)
> +static __always_inline void _tlbie_va(unsigned long va, unsigned long pid,
> + unsigned long psize, unsigned long ric)
> {
> unsigned long ap = mmu_get_ap(psize);
>
> @@ -424,7 +424,7 @@ static inline void _tlbie_va(unsigned long va, unsigned long pid,
> asm volatile("eieio; tlbsync; ptesync": : :"memory");
> }
>
> -static inline void _tlbie_lpid_va(unsigned long va, unsigned long lpid,
> +static __always_inline void _tlbie_lpid_va(unsigned long va, unsigned long lpid,
> unsigned long psize, unsigned long ric)
> {
> unsigned long ap = mmu_get_ap(psize);
>
^ permalink raw reply
* Re: [PATCH v3 1/2] pid: add pidfd_open()
From: Christian Brauner @ 2019-05-21 14:32 UTC (permalink / raw)
To: jannh, oleg, viro, torvalds, linux-kernel, arnd
Cc: linux-ia64, linux-sh, linux-mips, dhowells, joel, linux-kselftest,
sparclinux, elena.reshetova, linux-arch, linux-s390, dancol,
kernel-team, serge, linux-xtensa, keescook, linux-m68k, luto,
tglx, surenb, linux-arm-kernel, linux-parisc, linux-api, cyphar,
luto, ebiederm, linux-alpha, akpm, linuxppc-dev
In-Reply-To: <20190520155630.21684-1-christian@brauner.io>
On Mon, May 20, 2019 at 05:56:29PM +0200, Christian Brauner wrote:
> This adds the pidfd_open() syscall. It allows a caller to retrieve pollable
> pidfds for a process which did not get created via CLONE_PIDFD, i.e. for a
> process that is created via traditional fork()/clone() calls that is only
> referenced by a PID:
>
> int pidfd = pidfd_open(1234, 0);
> ret = pidfd_send_signal(pidfd, SIGSTOP, NULL, 0);
>
> With the introduction of pidfds through CLONE_PIDFD it is possible to
> created pidfds at process creation time.
> However, a lot of processes get created with traditional PID-based calls
> such as fork() or clone() (without CLONE_PIDFD). For these processes a
> caller can currently not create a pollable pidfd. This is a problem for
> Android's low memory killer (LMK) and service managers such as systemd.
> Both are examples of tools that want to make use of pidfds to get reliable
> notification of process exit for non-parents (pidfd polling) and race-free
> signal sending (pidfd_send_signal()). They intend to switch to this API for
> process supervision/management as soon as possible. Having no way to get
> pollable pidfds from PID-only processes is one of the biggest blockers for
> them in adopting this api. With pidfd_open() making it possible to retrieve
> pidfds for PID-based processes we enable them to adopt this api.
>
> In line with Arnd's recent changes to consolidate syscall numbers across
> architectures, I have added the pidfd_open() syscall to all architectures
> at the same time.
>
> Signed-off-by: Christian Brauner <christian@brauner.io>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
This now also carries a Reviewed-by from David.
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jann Horn <jannh@google.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Andy Lutomirsky <luto@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-api@vger.kernel.org
I've moved pidfd_open() into my for-next branch together with Joel's
pidfd polling changes. Everything is based on v5.2-rc1.
The chosen syscall number for now is 434. David is going to send out
another pile of mount api related syscalls. I'll coordinate with him
accordingly prior to the 5.3 merge window.
Thanks!
Christian
^ permalink raw reply
* RE: [PATCH] mm/nvdimm: Use correct #defines instead of opencoding
From: Elliott, Robert (Servers) @ 2019-05-21 14:49 UTC (permalink / raw)
To: Aneesh Kumar K.V, Dan Williams; +Cc: Linux MM, linuxppc-dev, linux-nvdimm
In-Reply-To: <87mujgcf0h.fsf@linux.ibm.com>
> -----Original Message-----
> From: Linux-nvdimm <linux-nvdimm-bounces@lists.01.org> On Behalf Of
> Aneesh Kumar K.V
> Sent: Tuesday, May 21, 2019 4:51 AM
> Subject: Re: [PATCH] mm/nvdimm: Use correct #defines instead of
> opencoding
>
...
> @@ -36,6 +36,9 @@ struct nd_pfn_sb {
> __le32 end_trunc;
> /* minor-version-2 record the base alignment of the mapping */
> __le32 align;
> + /* minor-version-3 record the page size and struct page size
> */
> + __le32 page_size;
> + __le32 page_struct_size;
> u8 padding[4000];
> __le64 checksum;
> };
You might need to reduce the padding size to offset the extra added
fields.
^ permalink raw reply
* Re: [PATCH 1/2] open: add close_range()
From: Al Viro @ 2019-05-21 15:00 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-ia64, linux-sh, oleg, dhowells, linux-kselftest, sparclinux,
shuah, linux-arch, linux-s390, miklos, x86, torvalds, linux-mips,
linux-xtensa, tkjos, arnd, jannh, linux-m68k, tglx, ldv,
linux-arm-kernel, fweimer, linux-parisc, linux-api, linux-kernel,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190521113448.20654-1-christian@brauner.io>
On Tue, May 21, 2019 at 01:34:47PM +0200, Christian Brauner wrote:
> This adds the close_range() syscall. It allows to efficiently close a range
> of file descriptors up to all file descriptors of a calling task.
>
> The syscall came up in a recent discussion around the new mount API and
> making new file descriptor types cloexec by default. During this
> discussion, Al suggested the close_range() syscall (cf. [1]). Note, a
> syscall in this manner has been requested by various people over time.
>
> First, it helps to close all file descriptors of an exec()ing task. This
> can be done safely via (quoting Al's example from [1] verbatim):
>
> /* that exec is sensitive */
> unshare(CLONE_FILES);
> /* we don't want anything past stderr here */
> close_range(3, ~0U);
> execve(....);
>
> The code snippet above is one way of working around the problem that file
> descriptors are not cloexec by default. This is aggravated by the fact that
> we can't just switch them over without massively regressing userspace. For
> a whole class of programs having an in-kernel method of closing all file
> descriptors is very helpful (e.g. demons, service managers, programming
> language standard libraries, container managers etc.).
> (Please note, unshare(CLONE_FILES) should only be needed if the calling
> task is multi-threaded and shares the file descriptor table with another
> thread in which case two threads could race with one thread allocating
> file descriptors and the other one closing them via close_range(). For the
> general case close_range() before the execve() is sufficient.)
>
> Second, it allows userspace to avoid implementing closing all file
> descriptors by parsing through /proc/<pid>/fd/* and calling close() on each
> file descriptor. From looking at various large(ish) userspace code bases
> this or similar patterns are very common in:
> - service managers (cf. [4])
> - libcs (cf. [6])
> - container runtimes (cf. [5])
> - programming language runtimes/standard libraries
> - Python (cf. [2])
> - Rust (cf. [7], [8])
> As Dmitry pointed out there's even a long-standing glibc bug about missing
> kernel support for this task (cf. [3]).
> In addition, the syscall will also work for tasks that do not have procfs
> mounted and on kernels that do not have procfs support compiled in. In such
> situations the only way to make sure that all file descriptors are closed
> is to call close() on each file descriptor up to UINT_MAX or RLIMIT_NOFILE,
> OPEN_MAX trickery (cf. comment [8] on Rust).
>
> The performance is striking. For good measure, comparing the following
> simple close_all_fds() userspace implementation that is essentially just
> glibc's version in [6]:
>
> static int close_all_fds(void)
> {
> DIR *dir;
> struct dirent *direntp;
>
> dir = opendir("/proc/self/fd");
> if (!dir)
> return -1;
>
> while ((direntp = readdir(dir))) {
> int fd;
> if (strcmp(direntp->d_name, ".") == 0)
> continue;
> if (strcmp(direntp->d_name, "..") == 0)
> continue;
> fd = atoi(direntp->d_name);
> if (fd == 0 || fd == 1 || fd == 2)
> continue;
> close(fd);
> }
>
> closedir(dir); /* cannot fail */
> return 0;
> }
>
> to close_range() yields:
> 1. closing 4 open files:
> - close_all_fds(): ~280 us
> - close_range(): ~24 us
>
> 2. closing 1000 open files:
> - close_all_fds(): ~5000 us
> - close_range(): ~800 us
>
> close_range() is designed to allow for some flexibility. Specifically, it
> does not simply always close all open file descriptors of a task. Instead,
> callers can specify an upper bound.
> This is e.g. useful for scenarios where specific file descriptors are
> created with well-known numbers that are supposed to be excluded from
> getting closed.
> For extra paranoia close_range() comes with a flags argument. This can e.g.
> be used to implement extension. Once can imagine userspace wanting to stop
> at the first error instead of ignoring errors under certain circumstances.
> There might be other valid ideas in the future. In any case, a flag
> argument doesn't hurt and keeps us on the safe side.
>
> >From an implementation side this is kept rather dumb. It saw some input
> from David and Jann but all nonsense is obviously my own!
> - Errors to close file descriptors are currently ignored. (Could be changed
> by setting a flag in the future if needed.)
> - __close_range() is a rather simplistic wrapper around __close_fd().
> My reasoning behind this is based on the nature of how __close_fd() needs
> to release an fd. But maybe I misunderstood specifics:
> We take the files_lock and rcu-dereference the fdtable of the calling
> task, we find the entry in the fdtable, get the file and need to release
> files_lock before calling filp_close().
> In the meantime the fdtable might have been altered so we can't just
> retake the spinlock and keep the old rcu-reference of the fdtable
> around. Instead we need to grab a fresh reference to the fdtable.
> If my reasoning is correct then there's really no point in fancyfying
> __close_range(): We just need to rcu-dereference the fdtable of the
> calling task once to cap the max_fd value correctly and then go on
> calling __close_fd() in a loop.
> +/**
> + * __close_range() - Close all file descriptors in a given range.
> + *
> + * @fd: starting file descriptor to close
> + * @max_fd: last file descriptor to close
> + *
> + * This closes a range of file descriptors. All file descriptors
> + * from @fd up to and including @max_fd are closed.
> + */
> +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> +{
> + unsigned int cur_max;
> +
> + if (fd > max_fd)
> + return -EINVAL;
> +
> + rcu_read_lock();
> + cur_max = files_fdtable(files)->max_fds;
> + rcu_read_unlock();
> +
> + /* cap to last valid index into fdtable */
> + if (max_fd >= cur_max)
> + max_fd = cur_max - 1;
> +
> + while (fd <= max_fd)
> + __close_fd(files, fd++);
> +
> + return 0;
> +}
Umm... That's going to be very painful if you dup2() something to MAX_INT and
then run that; roughly 2G iterations of bouncing ->file_lock up and down,
without anything that would yield CPU in process.
If anything, I would suggest something like
fd = *start_fd;
grab the lock
fdt = files_fdtable(files);
more:
look for the next eviction candidate in ->open_fds, starting at fd
if there's none up to max_fd
drop the lock
return NULL
*start_fd = fd + 1;
if the fscker is really opened and not just reserved
rcu_assign_pointer(fdt->fd[fd], NULL);
__put_unused_fd(files, fd);
drop the lock
return the file we'd got
if (unlikely(need_resched()))
drop lock
cond_resched();
grab lock
fdt = files_fdtable(files);
goto more;
with the main loop being basically
while ((file = pick_next(files, &start_fd, max_fd)) != NULL)
filp_close(file, files);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox