Hello Casey Schaufler,
Commit 95a3c11eb670 ("lsm: replace context+len with lsm_context")
from Oct 23, 2024 (linux-next), leads to the following Smatch static
checker warning:
net/netfilter/nfnetlink_queue.c:646 nfqnl_build_packet_message()
warn: always true condition '(seclen >= 0) => (0-u32max >= 0)'
net/netfilter/nfnetlink_queue.c:813 nfqnl_build_packet_message()
warn: always true condition '(seclen >= 0) => (0-u32max >= 0)'
net/netfilter/nfnetlink_queue.c:822 nfqnl_build_packet_message()
warn: always true condition '(seclen >= 0) => (0-u32max >= 0)'
net/netfilter/nfnetlink_queue.c
551 static struct sk_buff *
552 nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
553 struct nf_queue_entry *entry,
554 __be32 **packet_id_ptr)
555 {
556 size_t size;
557 size_t data_len = 0, cap_len = 0;
558 unsigned int hlen = 0;
559 struct sk_buff *skb;
560 struct nlattr *nla;
561 struct nfqnl_msg_packet_hdr *pmsg;
562 struct nlmsghdr *nlh;
563 struct sk_buff *entskb = entry->skb;
564 struct net_device *indev;
565 struct net_device *outdev;
566 struct nf_conn *ct = NULL;
567 enum ip_conntrack_info ctinfo = 0;
568 const struct nfnl_ct_hook *nfnl_ct;
569 bool csum_verify;
570 struct lsm_context ctx;
571 u32 seclen = 0;
^^^^^^^^^^
572 ktime_t tstamp;
573
574 size = nlmsg_total_size(sizeof(struct nfgenmsg))
575 + nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
576 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
577 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
578 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
579 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
580 + nla_total_size(sizeof(u_int32_t)) /* ifindex */
581 #endif
582 + nla_total_size(sizeof(u_int32_t)) /* mark */
583 + nla_total_size(sizeof(u_int32_t)) /* priority */
584 + nla_total_size(sizeof(struct nfqnl_msg_packet_hw))
585 + nla_total_size(sizeof(u_int32_t)) /* skbinfo */
586 #if IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)
587 + nla_total_size(sizeof(u_int32_t)) /* classid */
588 #endif
589 + nla_total_size(sizeof(u_int32_t)); /* cap_len */
590
591 tstamp = skb_tstamp_cond(entskb, false);
592 if (tstamp)
593 size += nla_total_size(sizeof(struct nfqnl_msg_packet_timestamp));
594
595 size += nfqnl_get_bridge_size(entry);
596
597 if (entry->state.hook <= NF_INET_FORWARD ||
598 (entry->state.hook == NF_INET_POST_ROUTING && entskb->sk == NULL))
599 csum_verify = !skb_csum_unnecessary(entskb);
600 else
601 csum_verify = false;
602
603 outdev = entry->state.out;
604
605 switch ((enum nfqnl_config_mode)READ_ONCE(queue->copy_mode)) {
606 case NFQNL_COPY_META:
607 case NFQNL_COPY_NONE:
608 break;
609
610 case NFQNL_COPY_PACKET:
611 if (!(queue->flags & NFQA_CFG_F_GSO) &&
612 entskb->ip_summed == CHECKSUM_PARTIAL &&
613 nf_queue_checksum_help(entskb))
614 return NULL;
615
616 data_len = READ_ONCE(queue->copy_range);
617 if (data_len > entskb->len)
618 data_len = entskb->len;
619
620 hlen = skb_zerocopy_headlen(entskb);
621 hlen = min_t(unsigned int, hlen, data_len);
622 size += sizeof(struct nlattr) + hlen;
623 cap_len = entskb->len;
624 break;
625 }
626
627 nfnl_ct = rcu_dereference(nfnl_ct_hook);
628
629 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
630 if (queue->flags & NFQA_CFG_F_CONNTRACK) {
631 if (nfnl_ct != NULL) {
632 ct = nf_ct_get(entskb, &ctinfo);
633 if (ct != NULL)
634 size += nfnl_ct->build_size(ct);
635 }
636 }
637 #endif
638
639 if (queue->flags & NFQA_CFG_F_UID_GID) {
640 size += (nla_total_size(sizeof(u_int32_t)) /* uid */
641 + nla_total_size(sizeof(u_int32_t))); /* gid */
642 }
643
644 if ((queue->flags & NFQA_CFG_F_SECCTX) && entskb->sk) {
645 seclen = nfqnl_get_sk_secctx(entskb, &ctx);
nfqnl_get_sk_secctx() returns negative error codes. It needs to be changed ot
int as well instead of u32.
646 if (seclen >= 0)
647 size += nla_total_size(seclen);
648 }
649
650 skb = alloc_skb(size, GFP_ATOMIC);
651 if (!skb) {
652 skb_tx_error(entskb);
653 goto nlmsg_failure;
654 }
655
656 nlh = nfnl_msg_put(skb, 0, 0,
657 nfnl_msg_type(NFNL_SUBSYS_QUEUE, NFQNL_MSG_PACKET),
658 0, entry->state.pf, NFNETLINK_V0,
659 htons(queue->queue_num));
660 if (!nlh) {
661 skb_tx_error(entskb);
662 kfree_skb(skb);
663 goto nlmsg_failure;
664 }
665
666 nla = __nla_reserve(skb, NFQA_PACKET_HDR, sizeof(*pmsg));
667 pmsg = nla_data(nla);
668 pmsg->hw_protocol = entskb->protocol;
669 pmsg->hook = entry->state.hook;
670 *packet_id_ptr = &pmsg->packet_id;
671
672 indev = entry->state.in;
673 if (indev) {
674 #if !IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
675 if (nla_put_be32(skb, NFQA_IFINDEX_INDEV, htonl(indev->ifindex)))
676 goto nla_put_failure;
677 #else
678 if (entry->state.pf == PF_BRIDGE) {
679 /* Case 1: indev is physical input device, we need to
680 * look for bridge group (when called from
681 * netfilter_bridge) */
682 if (nla_put_be32(skb, NFQA_IFINDEX_PHYSINDEV,
683 htonl(indev->ifindex)) ||
684 /* this is the bridge group "brX" */
685 /* rcu_read_lock()ed by __nf_queue */
686 nla_put_be32(skb, NFQA_IFINDEX_INDEV,
687 htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
688 goto nla_put_failure;
689 } else {
690 int physinif;
691
692 /* Case 2: indev is bridge group, we need to look for
693 * physical device (when called from ipv4) */
694 if (nla_put_be32(skb, NFQA_IFINDEX_INDEV,
695 htonl(indev->ifindex)))
696 goto nla_put_failure;
697
698 physinif = nf_bridge_get_physinif(entskb);
699 if (physinif &&
700 nla_put_be32(skb, NFQA_IFINDEX_PHYSINDEV,
701 htonl(physinif)))
702 goto nla_put_failure;
703 }
704 #endif
705 }
706
707 if (outdev) {
708 #if !IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
709 if (nla_put_be32(skb, NFQA_IFINDEX_OUTDEV, htonl(outdev->ifindex)))
710 goto nla_put_failure;
711 #else
712 if (entry->state.pf == PF_BRIDGE) {
713 /* Case 1: outdev is physical output device, we need to
714 * look for bridge group (when called from
715 * netfilter_bridge) */
716 if (nla_put_be32(skb, NFQA_IFINDEX_PHYSOUTDEV,
717 htonl(outdev->ifindex)) ||
718 /* this is the bridge group "brX" */
719 /* rcu_read_lock()ed by __nf_queue */
720 nla_put_be32(skb, NFQA_IFINDEX_OUTDEV,
721 htonl(br_port_get_rcu(outdev)->br->dev->ifindex)))
722 goto nla_put_failure;
723 } else {
724 int physoutif;
725
726 /* Case 2: outdev is bridge group, we need to look for
727 * physical output device (when called from ipv4) */
728 if (nla_put_be32(skb, NFQA_IFINDEX_OUTDEV,
729 htonl(outdev->ifindex)))
730 goto nla_put_failure;
731
732 physoutif = nf_bridge_get_physoutif(entskb);
733 if (physoutif &&
734 nla_put_be32(skb, NFQA_IFINDEX_PHYSOUTDEV,
735 htonl(physoutif)))
736 goto nla_put_failure;
737 }
738 #endif
739 }
740
741 if (entskb->mark &&
742 nla_put_be32(skb, NFQA_MARK, htonl(entskb->mark)))
743 goto nla_put_failure;
744
745 if (entskb->priority &&
746 nla_put_be32(skb, NFQA_PRIORITY, htonl(entskb->priority)))
747 goto nla_put_failure;
748
749 if (indev && entskb->dev &&
750 skb_mac_header_was_set(entskb) &&
751 skb_mac_header_len(entskb) != 0) {
752 struct nfqnl_msg_packet_hw phw;
753 int len;
754
755 memset(&phw, 0, sizeof(phw));
756 len = dev_parse_header(entskb, phw.hw_addr);
757 if (len) {
758 phw.hw_addrlen = htons(len);
759 if (nla_put(skb, NFQA_HWADDR, sizeof(phw), &phw))
760 goto nla_put_failure;
761 }
762 }
763
764 if (nfqnl_put_bridge(entry, skb) < 0)
765 goto nla_put_failure;
766
767 if (entry->state.hook <= NF_INET_FORWARD && tstamp) {
768 struct nfqnl_msg_packet_timestamp ts;
769 struct timespec64 kts = ktime_to_timespec64(tstamp);
770
771 ts.sec = cpu_to_be64(kts.tv_sec);
772 ts.usec = cpu_to_be64(kts.tv_nsec / NSEC_PER_USEC);
773
774 if (nla_put(skb, NFQA_TIMESTAMP, sizeof(ts), &ts))
775 goto nla_put_failure;
776 }
777
778 if ((queue->flags & NFQA_CFG_F_UID_GID) && entskb->sk &&
779 nfqnl_put_sk_uidgid(skb, entskb->sk) < 0)
780 goto nla_put_failure;
781
782 if (nfqnl_put_sk_classid(skb, entskb->sk) < 0)
783 goto nla_put_failure;
784
785 if (seclen && nla_put(skb, NFQA_SECCTX, ctx.len, ctx.context))
^^^^^^
This doesn't look right.
786 goto nla_put_failure;
787
788 if (ct && nfnl_ct->build(skb, ct, ctinfo, NFQA_CT, NFQA_CT_INFO) < 0)
789 goto nla_put_failure;
790
791 if (cap_len > data_len &&
792 nla_put_be32(skb, NFQA_CAP_LEN, htonl(cap_len)))
793 goto nla_put_failure;
794
795 if (nfqnl_put_packet_info(skb, entskb, csum_verify))
796 goto nla_put_failure;
797
798 if (data_len) {
799 struct nlattr *nla;
800
801 if (skb_tailroom(skb) < sizeof(*nla) + hlen)
802 goto nla_put_failure;
803
804 nla = skb_put(skb, sizeof(*nla));
805 nla->nla_type = NFQA_PAYLOAD;
806 nla->nla_len = nla_attr_size(data_len);
807
808 if (skb_zerocopy(skb, entskb, data_len, hlen))
809 goto nla_put_failure;
810 }
811
812 nlh->nlmsg_len = skb->len;
813 if (seclen >= 0)
^^^^^^^^^^^
814 security_release_secctx(&ctx);
815 return skb;
816
817 nla_put_failure:
818 skb_tx_error(entskb);
819 kfree_skb(skb);
820 net_err_ratelimited("nf_queue: error creating packet message\n");
821 nlmsg_failure:
822 if (seclen >= 0)
^^^^^^^^^^^
823 security_release_secctx(&ctx);
824 return NULL;
825 }
regards,
dan carpenter
