| Message ID | 20200805122501.4856-1-r.czerwinski@pengutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/2] net: tls: add compat for get/setsockopt | expand |
Neither of these patches apply cleanly to net-next. The compat handling and TLS code has been changed quite a bit lately. ALso, you must provide a proper header "[PATCH 0/N] ..." posting for your patch series which explains at a high level what your patch series is doing, how it is doing it, and why it is doing it that way. Thank you.
Hi Rouven,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.8]
[cannot apply to next-20200805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rouven-Czerwinski/net-tls-add-compat-for-get-setsockopt/20200806-040123
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git ecfd7940b8641da6e41ca94eba36876dc2ba827b
config: i386-randconfig-s002-20200805 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-117-g8c7aee71-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/tls/tls_main.c: In function 'tls_compat_getsockopt':
>> net/tls/tls_main.c:459:23: error: 'struct proto' has no member named 'compat_getsockopt'
459 | return ctx->sk_proto->compat_getsockopt(sk, level, optname,
| ^~
net/tls/tls_main.c: In function 'tls_compat_setsockopt':
>> net/tls/tls_main.c:632:23: error: 'struct proto' has no member named 'compat_setsockopt'
632 | return ctx->sk_proto->compat_setsockopt(sk, level, optname,
| ^~
At top level:
net/tls/tls_main.c:626:12: warning: 'tls_compat_setsockopt' defined but not used [-Wunused-function]
626 | static int tls_compat_setsockopt(struct sock *sk, int level, int optname,
| ^~~~~~~~~~~~~~~~~~~~~
net/tls/tls_main.c:453:12: warning: 'tls_compat_getsockopt' defined but not used [-Wunused-function]
453 | static int tls_compat_getsockopt(struct sock *sk, int level, int optname,
| ^~~~~~~~~~~~~~~~~~~~~
vim +459 net/tls/tls_main.c
452
453 static int tls_compat_getsockopt(struct sock *sk, int level, int optname,
454 char __user *optval, int __user *optlen)
455 {
456 struct tls_context *ctx = tls_get_ctx(sk);
457
458 if (level != SOL_TLS)
> 459 return ctx->sk_proto->compat_getsockopt(sk, level, optname,
460 optval, optlen);
461
462 return do_tls_getsockopt(sk, optname, optval, optlen);
463 }
464
465 static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
466 unsigned int optlen, int tx)
467 {
468 struct tls_crypto_info *crypto_info;
469 struct tls_crypto_info *alt_crypto_info;
470 struct tls_context *ctx = tls_get_ctx(sk);
471 size_t optsize;
472 int rc = 0;
473 int conf;
474
475 if (!optval || (optlen < sizeof(*crypto_info))) {
476 rc = -EINVAL;
477 goto out;
478 }
479
480 if (tx) {
481 crypto_info = &ctx->crypto_send.info;
482 alt_crypto_info = &ctx->crypto_recv.info;
483 } else {
484 crypto_info = &ctx->crypto_recv.info;
485 alt_crypto_info = &ctx->crypto_send.info;
486 }
487
488 /* Currently we don't support set crypto info more than one time */
489 if (TLS_CRYPTO_INFO_READY(crypto_info)) {
490 rc = -EBUSY;
491 goto out;
492 }
493
494 rc = copy_from_user(crypto_info, optval, sizeof(*crypto_info));
495 if (rc) {
496 rc = -EFAULT;
497 goto err_crypto_info;
498 }
499
500 /* check version */
501 if (crypto_info->version != TLS_1_2_VERSION &&
502 crypto_info->version != TLS_1_3_VERSION) {
503 rc = -EINVAL;
504 goto err_crypto_info;
505 }
506
507 /* Ensure that TLS version and ciphers are same in both directions */
508 if (TLS_CRYPTO_INFO_READY(alt_crypto_info)) {
509 if (alt_crypto_info->version != crypto_info->version ||
510 alt_crypto_info->cipher_type != crypto_info->cipher_type) {
511 rc = -EINVAL;
512 goto err_crypto_info;
513 }
514 }
515
516 switch (crypto_info->cipher_type) {
517 case TLS_CIPHER_AES_GCM_128:
518 optsize = sizeof(struct tls12_crypto_info_aes_gcm_128);
519 break;
520 case TLS_CIPHER_AES_GCM_256: {
521 optsize = sizeof(struct tls12_crypto_info_aes_gcm_256);
522 break;
523 }
524 case TLS_CIPHER_AES_CCM_128:
525 optsize = sizeof(struct tls12_crypto_info_aes_ccm_128);
526 break;
527 default:
528 rc = -EINVAL;
529 goto err_crypto_info;
530 }
531
532 if (optlen != optsize) {
533 rc = -EINVAL;
534 goto err_crypto_info;
535 }
536
537 rc = copy_from_user(crypto_info + 1, optval + sizeof(*crypto_info),
538 optlen - sizeof(*crypto_info));
539 if (rc) {
540 rc = -EFAULT;
541 goto err_crypto_info;
542 }
543
544 if (tx) {
545 rc = tls_set_device_offload(sk, ctx);
546 conf = TLS_HW;
547 if (!rc) {
548 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSTXDEVICE);
549 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRTXDEVICE);
550 } else {
551 rc = tls_set_sw_offload(sk, ctx, 1);
552 if (rc)
553 goto err_crypto_info;
554 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSTXSW);
555 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRTXSW);
556 conf = TLS_SW;
557 }
558 } else {
559 rc = tls_set_device_offload_rx(sk, ctx);
560 conf = TLS_HW;
561 if (!rc) {
562 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSRXDEVICE);
563 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRRXDEVICE);
564 } else {
565 rc = tls_set_sw_offload(sk, ctx, 0);
566 if (rc)
567 goto err_crypto_info;
568 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSRXSW);
569 TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRRXSW);
570 conf = TLS_SW;
571 }
572 tls_sw_strparser_arm(sk, ctx);
573 }
574
575 if (tx)
576 ctx->tx_conf = conf;
577 else
578 ctx->rx_conf = conf;
579 update_sk_prot(sk, ctx);
580 if (tx) {
581 ctx->sk_write_space = sk->sk_write_space;
582 sk->sk_write_space = tls_write_space;
583 } else {
584 sk->sk_socket->ops = &tls_sw_proto_ops;
585 }
586 goto out;
587
588 err_crypto_info:
589 memzero_explicit(crypto_info, sizeof(union tls_crypto_context));
590 out:
591 return rc;
592 }
593
594 static int do_tls_setsockopt(struct sock *sk, int optname,
595 char __user *optval, unsigned int optlen)
596 {
597 int rc = 0;
598
599 switch (optname) {
600 case TLS_TX:
601 case TLS_RX:
602 lock_sock(sk);
603 rc = do_tls_setsockopt_conf(sk, optval, optlen,
604 optname == TLS_TX);
605 release_sock(sk);
606 break;
607 default:
608 rc = -ENOPROTOOPT;
609 break;
610 }
611 return rc;
612 }
613
614 static int tls_setsockopt(struct sock *sk, int level, int optname,
615 char __user *optval, unsigned int optlen)
616 {
617 struct tls_context *ctx = tls_get_ctx(sk);
618
619 if (level != SOL_TLS)
620 return ctx->sk_proto->setsockopt(sk, level, optname, optval,
621 optlen);
622
623 return do_tls_setsockopt(sk, optname, optval, optlen);
624 }
625
626 static int tls_compat_setsockopt(struct sock *sk, int level, int optname,
627 char __user *optval, unsigned int optlen)
628 {
629 struct tls_context *ctx = tls_get_ctx(sk);
630
631 if (level != SOL_TLS)
> 632 return ctx->sk_proto->compat_setsockopt(sk, level, optname,
633 optval, optlen);
634
635 return do_tls_setsockopt(sk, optname, optval, optlen);
636 }
637
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, 2020-08-05 at 17:45 -0700, David Miller wrote: > Neither of these patches apply cleanly to net-next. The compat handling > and TLS code has been changed quite a bit lately. Indeed, Patch 1 is no longer required on net-next. I'll drop the patch. > ALso, you must provide a proper header "[PATCH 0/N] ..." posting for your > patch series which explains at a high level what your patch series is doing, > how it is doing it, and why it is doing it that way. Since I'm now down to one patch I'll forgo the cover letter and expand the commit message. Thanks for the explanation, Rouven
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index ec10041c6b7d..92c5893fe692 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -450,6 +450,18 @@ static int tls_getsockopt(struct sock *sk, int level, int optname, return do_tls_getsockopt(sk, optname, optval, optlen); } +static int tls_compat_getsockopt(struct sock *sk, int level, int optname, + char __user *optval, int __user *optlen) +{ + struct tls_context *ctx = tls_get_ctx(sk); + + if (level != SOL_TLS) + return ctx->sk_proto->compat_getsockopt(sk, level, optname, + optval, optlen); + + return do_tls_getsockopt(sk, optname, optval, optlen); +} + static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval, unsigned int optlen, int tx) { @@ -611,6 +623,18 @@ static int tls_setsockopt(struct sock *sk, int level, int optname, return do_tls_setsockopt(sk, optname, optval, optlen); } +static int tls_compat_setsockopt(struct sock *sk, int level, int optname, + char __user *optval, unsigned int optlen) +{ + struct tls_context *ctx = tls_get_ctx(sk); + + if (level != SOL_TLS) + return ctx->sk_proto->compat_setsockopt(sk, level, optname, + optval, optlen); + + return do_tls_setsockopt(sk, optname, optval, optlen); +} + struct tls_context *tls_ctx_create(struct sock *sk) { struct inet_connection_sock *icsk = inet_csk(sk); @@ -660,6 +684,10 @@ static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG], prot[TLS_BASE][TLS_BASE].setsockopt = tls_setsockopt; prot[TLS_BASE][TLS_BASE].getsockopt = tls_getsockopt; prot[TLS_BASE][TLS_BASE].close = tls_sk_proto_close; +#ifdef CONFIG_COMPAT + prot[TLS_BASE][TLS_BASE].compat_setsockopt = tls_compat_setsockopt; + prot[TLS_BASE][TLS_BASE].compat_getsockopt = tls_compat_getsockopt; +#endif prot[TLS_SW][TLS_BASE] = prot[TLS_BASE][TLS_BASE]; prot[TLS_SW][TLS_BASE].sendmsg = tls_sw_sendmsg;
If compat_{s,g}etsockopt for TLS are not implemented, the TLS layer will never be called on a system where CONFIG_COMPAT is enabled and userspace is 32bit. Implement both to support CONFIG_COMPAT. Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de> --- net/tls/tls_main.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)