Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
Arnd Bergmann wrote: > From what I can tell, neither of the two are called in atomic context, so > you should be able to use a GFP_KERNEL allocation. You need to be careful doing that since the allocation might happen in the AFS writeback path. I use GFP_NOIO or GFP_NOFS in rxkad.c and skb_cow_data() uses GFP_ATOMIC - though we should have single ownership of the packet at this point. David -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
On Thu, Jul 12, 2018 at 2:15 PM, Arnd Bergmann wrote: > On Thu, Jul 12, 2018 at 10:30 PM, Kees Cook wrote: >> Actually, I think this can actually be adjusted to just re-use the >> stack allocation, since rxkad_verify_packet() finishes one before >> doing another in rxkad_verify_packet_1(): > > That looks very nice, yes. The same thing is needed in > rxkad_secure_packet(), right? Yup. 4 leaf functions and the 2 callers. -Kees -- Kees Cook Pixel Security -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
Hi David, On Thu, Jul 12, 2018 at 2:28 PM, David Howells wrote: > Can I get a cc on the original patch? I'll add you to CC for future revisions. Here was the start of this thread: https://lkml.kernel.org/r/20180711203619.1020-14-keesc...@chromium.org -Kees -- Kees Cook Pixel Security -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
Can I get a cc on the original patch? David -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
On Thu, Jul 12, 2018 at 10:30 PM, Kees Cook wrote: > On Thu, Jul 12, 2018 at 1:23 PM, Kees Cook wrote: >> On Thu, Jul 12, 2018 at 8:11 AM, Arnd Bergmann wrote: >>> On Wed, Jul 11, 2018 at 10:36 PM, Kees Cook wrote: Two uses of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings (when less than 2048) once the VLA is no longer hidden from the check: net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] This bumps the affected objects by 20% to silence the warnings while still providing coverage is anything grows even more. Signed-off-by: Kees Cook >>> >>> (adding David Howells to cc) >>> >>> I don't think these are in a fast path, it should be possible to just use >>> skcipher_alloc_req() instead of SKCIPHER_REQUEST_ON_STACK() here. >>> From what I can tell, neither of the two are called in atomic context, so >>> you should be able to use a GFP_KERNEL allocation. >> >> Sure, I can do that instead. > > Actually, I think this can actually be adjusted to just re-use the > stack allocation, since rxkad_verify_packet() finishes one before > doing another in rxkad_verify_packet_1(): That looks very nice, yes. The same thing is needed in rxkad_secure_packet(), right? Arnd -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
On Thu, Jul 12, 2018 at 1:23 PM, Kees Cook wrote: > On Thu, Jul 12, 2018 at 8:11 AM, Arnd Bergmann wrote: >> On Wed, Jul 11, 2018 at 10:36 PM, Kees Cook wrote: >>> Two uses of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings >>> (when less than 2048) once the VLA is no longer hidden from the check: >>> >>> net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger >>> than 1024 bytes [-Wframe-larger-than=] >>> net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger >>> than 1024 bytes [-Wframe-larger-than=] >>> >>> This bumps the affected objects by 20% to silence the warnings while >>> still providing coverage is anything grows even more. >>> >>> Signed-off-by: Kees Cook >> >> (adding David Howells to cc) >> >> I don't think these are in a fast path, it should be possible to just use >> skcipher_alloc_req() instead of SKCIPHER_REQUEST_ON_STACK() here. >> From what I can tell, neither of the two are called in atomic context, so >> you should be able to use a GFP_KERNEL allocation. > > Sure, I can do that instead. Actually, I think this can actually be adjusted to just re-use the stack allocation, since rxkad_verify_packet() finishes one before doing another in rxkad_verify_packet_1(): diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index 278ac0807a60..d6a2e7cab384 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -316,10 +316,10 @@ static int rxkad_secure_packet(struct rxrpc_call *call, */ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb, unsigned int offset, unsigned int len, -rxrpc_seq_t seq) +rxrpc_seq_t seq, +struct skcipher_request *req) { struct rxkad_level1_hdr sechdr; - SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher); struct rxrpc_crypt iv; struct scatterlist sg[16]; struct sk_buff *trailer; @@ -549,7 +549,7 @@ static int rxkad_verify_packet(struct rxrpc_call *call, struct sk_buff *skb, case RXRPC_SECURITY_PLAIN: return 0; case RXRPC_SECURITY_AUTH: - return rxkad_verify_packet_1(call, skb, offset, len, seq); + return rxkad_verify_packet_1(call, skb, offset, len, seq, req); case RXRPC_SECURITY_ENCRYPT: return rxkad_verify_packet_2(call, skb, offset, len, seq); default: -Kees -- Kees Cook Pixel Security -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
On Thu, Jul 12, 2018 at 8:11 AM, Arnd Bergmann wrote: > On Wed, Jul 11, 2018 at 10:36 PM, Kees Cook wrote: >> Two uses of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings >> (when less than 2048) once the VLA is no longer hidden from the check: >> >> net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger >> than 1024 bytes [-Wframe-larger-than=] >> net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger >> than 1024 bytes [-Wframe-larger-than=] >> >> This bumps the affected objects by 20% to silence the warnings while >> still providing coverage is anything grows even more. >> >> Signed-off-by: Kees Cook > > (adding David Howells to cc) > > I don't think these are in a fast path, it should be possible to just use > skcipher_alloc_req() instead of SKCIPHER_REQUEST_ON_STACK() here. > From what I can tell, neither of the two are called in atomic context, so > you should be able to use a GFP_KERNEL allocation. Sure, I can do that instead. -Kees -- Kees Cook Pixel Security -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
On Wed, Jul 11, 2018 at 10:36 PM, Kees Cook wrote: > Two uses of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings > (when less than 2048) once the VLA is no longer hidden from the check: > > net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger than > 1024 bytes [-Wframe-larger-than=] > net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger than > 1024 bytes [-Wframe-larger-than=] > > This bumps the affected objects by 20% to silence the warnings while > still providing coverage is anything grows even more. > > Signed-off-by: Kees Cook (adding David Howells to cc) I don't think these are in a fast path, it should be possible to just use skcipher_alloc_req() instead of SKCIPHER_REQUEST_ON_STACK() here. >From what I can tell, neither of the two are called in atomic context, so you should be able to use a GFP_KERNEL allocation. Arnd -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v4 13/14] rxrpc: Prepare to remove VLA usage for SKCIPHER_REQUEST_ON_STACK
Two uses of SKCIPHER_REQUEST_ON_STACK() will trigger FRAME_WARN warnings (when less than 2048) once the VLA is no longer hidden from the check: net/rxrpc/rxkad.c:398:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] net/rxrpc/rxkad.c:242:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] This bumps the affected objects by 20% to silence the warnings while still providing coverage is anything grows even more. Signed-off-by: Kees Cook --- net/rxrpc/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/net/rxrpc/Makefile b/net/rxrpc/Makefile index 6ffb7e9887ce..53e1177129e6 100644 --- a/net/rxrpc/Makefile +++ b/net/rxrpc/Makefile @@ -32,4 +32,5 @@ rxrpc-y := \ rxrpc-$(CONFIG_PROC_FS) += proc.o rxrpc-$(CONFIG_RXKAD) += rxkad.o +CFLAGS_rxkad.o += $(FRAME_WARN_BUMP_FLAG) rxrpc-$(CONFIG_SYSCTL) += sysctl.o -- 2.17.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel