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
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
