| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <201805031629.vXyGm7Ql%fengguang.wu@intel.com>
Date: Thu, 3 May 2018 16:26:44 +0800
From: kbuild test robot <lkp@...el.com>
To: Toke Høiland-Jørgensen <toke@...e.dk>
Cc: kbuild-all@...org, netdev@...r.kernel.org,
cake@...ts.bufferbloat.net
Subject: Re: [PATCH net-next v7 3/7] sch_cake: Add optional ACK filter
Hi Toke,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/sched-Add-Common-Applications-Kept-Enhanced-cake-qdisc/20180503-073002
coccinelle warnings: (new ones prefixed by >>)
>> net/sched/sch_cake.c:1047:6-13: ERROR: PTR_ERR applied after initialization to constant on line 822
vim +1047 net/sched/sch_cake.c
812
813 static struct sk_buff *cake_ack_filter(struct cake_sched_data *q,
814 struct cake_flow *flow)
815 {
816 bool thisconn_redundant_seen = false, thisconn_seen_last = false;
817 bool aggressive = q->ack_filter == CAKE_ACK_AGGRESSIVE;
818 bool otherconn_ack_seen = false;
819 struct sk_buff *skb_check, *skb_check_prev;
820 struct sk_buff *otherconn_checked_to = NULL;
821 struct sk_buff *thisconn_checked_to = NULL;
822 struct sk_buff *thisconn_ack = NULL;
823 const struct ipv6hdr *ipv6h, *ipv6h_check;
824 const struct tcphdr *tcph, *tcph_check;
825 const struct iphdr *iph, *iph_check;
826 const struct sk_buff *skb;
827 struct ipv6hdr _iph, _iph_check;
828 struct tcphdr _tcph_check;
829 unsigned char _tcph[64]; /* need to hold maximum hdr size */
830 int seglen;
831
832 /* no other possible ACKs to filter */
833 if (flow->head == flow->tail)
834 return NULL;
835
836 skb = flow->tail;
837 tcph = cake_get_tcphdr(skb, _tcph, sizeof(_tcph));
838 iph = cake_get_iphdr(skb, &_iph);
839 if (!tcph)
840 return NULL;
841
842 /* the 'triggering' packet need only have the ACK flag set.
843 * also check that SYN is not set, as there won't be any previous ACKs.
844 */
845 if ((tcp_flag_word(tcph) &
846 (TCP_FLAG_ACK | TCP_FLAG_SYN)) != TCP_FLAG_ACK)
847 return NULL;
848
849 /* the 'triggering' ACK is at the end of the queue,
850 * we have already returned if it is the only packet in the flow.
851 * stop before last packet in queue, don't compare trigger ACK to itself
852 * start where we finished last time if recorded in ->ackcheck
853 * otherwise start from the the head of the flow queue.
854 */
855 skb_check_prev = flow->ackcheck;
856 skb_check = flow->ackcheck ?: flow->head;
857
858 while (skb_check->next) {
859 bool pure_ack, thisconn;
860
861 /* don't increment if at head of flow queue (_prev == NULL) */
862 if (skb_check_prev) {
863 skb_check_prev = skb_check;
864 skb_check = skb_check->next;
865 if (!skb_check->next)
866 break;
867 } else {
868 skb_check_prev = ERR_PTR(-1);
869 }
870
871 iph_check = cake_get_iphdr(skb_check, &_iph_check);
872 tcph_check = cake_get_tcphdr(skb_check, &_tcph_check,
873 sizeof(_tcph_check));
874
875 if (!tcph_check || iph->version != iph_check->version)
876 continue;
877
878 if (iph->version == 4) {
879 seglen = ntohs(iph_check->tot_len) -
880 (4 * iph_check->ihl);
881
882 thisconn = (iph_check->saddr == iph->saddr &&
883 iph_check->daddr == iph->daddr);
884 } else if (iph->version == 6) {
885 ipv6h = (struct ipv6hdr *)iph;
886 ipv6h_check = (struct ipv6hdr *)iph_check;
887 seglen = ntohs(ipv6h_check->payload_len);
888
889 thisconn = (!ipv6_addr_cmp(&ipv6h_check->saddr,
890 &ipv6h->saddr) &&
891 !ipv6_addr_cmp(&ipv6h_check->daddr,
892 &ipv6h->daddr));
893 } else {
894 WARN_ON(1); /* shouldn't happen */
895 continue;
896 }
897
898 /* stricter criteria apply to ACKs that we may filter
899 * 3 reserved flags must be unset to avoid future breakage
900 * ECE/CWR/NS can be safely ignored
901 * ACK must be set
902 * All other flags URG/PSH/RST/SYN/FIN must be unset
903 * 0x0FFF0000 = all TCP flags (confirm ACK=1, others zero)
904 * 0x01C00000 = NS/CWR/ECE (safe to ignore)
905 * 0x0E3F0000 = 0x0FFF0000 & ~0x01C00000
906 * must be 'pure' ACK, contain zero bytes of segment data
907 * options are ignored
908 */
909 if ((tcp_flag_word(tcph_check) &
910 (TCP_FLAG_ACK | TCP_FLAG_SYN)) != TCP_FLAG_ACK)
911 continue;
912
913 else if (((tcp_flag_word(tcph_check) &
914 cpu_to_be32(0x0E3F0000)) != TCP_FLAG_ACK) ||
915 ((seglen - __tcp_hdrlen(tcph_check)) != 0))
916 pure_ack = false;
917
918 else
919 pure_ack = true;
920
921 /* if we find an ACK belonging to a different connection
922 * continue checking for other ACKs this round however
923 * restart checking from the other connection next time.
924 */
925 if (thisconn && (tcph_check->source != tcph->source ||
926 tcph_check->dest != tcph->dest))
927 thisconn = false;
928
929 /* new ack sequence must be greater
930 */
931 if (thisconn &&
932 ((int32_t)(ntohl(tcph_check->ack_seq) -
933 ntohl(tcph->ack_seq)) > 0))
934 continue;
935
936 /* DupACKs with an equal sequence number shouldn't be filtered,
937 * but we can filter if the triggering packet is a SACK
938 */
939 if (thisconn &&
940 (ntohl(tcph_check->ack_seq) == ntohl(tcph->ack_seq))) {
941 /* inspired by tcp_parse_options in tcp_input.c */
942 bool sack = false;
943 int length = __tcp_hdrlen(tcph) - sizeof(struct tcphdr);
944 const u8 *ptr = (const u8 *)(tcph + 1);
945
946 while (length > 0) {
947 int opcode = *ptr++;
948 int opsize;
949
950 if (opcode == TCPOPT_EOL)
951 break;
952 if (opcode == TCPOPT_NOP) {
953 length--;
954 continue;
955 }
956 opsize = *ptr++;
957 if (opsize < 2 || opsize > length)
958 break;
959 if (opcode == TCPOPT_SACK) {
960 sack = true;
961 break;
962 }
963 ptr += opsize - 2;
964 length -= opsize;
965 }
966 if (!sack)
967 continue;
968 }
969
970 /* somewhat complicated control flow for 'conservative'
971 * ACK filtering that aims to be more polite to slow-start and
972 * in the presence of packet loss.
973 * does not filter if there is one 'redundant' ACK in the queue.
974 * 'data' ACKs won't be filtered but do count as redundant ACKs.
975 */
976 if (thisconn) {
977 thisconn_seen_last = true;
978 /* if aggressive and this is a data ack we can skip
979 * checking it next time.
980 */
981 thisconn_checked_to = (aggressive && !pure_ack) ?
982 skb_check : skb_check_prev;
983 /* the first pure ack for this connection.
984 * record where it is, but only break if aggressive
985 * or already seen data ack from the same connection
986 */
987 if (pure_ack && !thisconn_ack) {
988 thisconn_ack = skb_check_prev;
989 if (aggressive || thisconn_redundant_seen)
990 break;
991 /* data ack or subsequent pure ack */
992 } else {
993 thisconn_redundant_seen = true;
994 /* this is the second ack for this connection
995 * break to filter the first pure ack
996 */
997 if (thisconn_ack)
998 break;
999 }
1000 /* track packets from non-matching tcp connections that will
1001 * need evaluation on the next run.
1002 * if there are packets from both the matching connection and
1003 * others that requre checking next run, track which was updated
1004 * last and return the older of the two to ensure full coverage.
1005 * if a non-matching pure ack has been seen, cannot skip any
1006 * further on the next run so don't update.
1007 */
1008 } else if (!otherconn_ack_seen) {
1009 thisconn_seen_last = false;
1010 if (pure_ack) {
1011 otherconn_ack_seen = true;
1012 /* if aggressive we don't care about old data,
1013 * start from the pure ack.
1014 * otherwise if there is a previous data ack,
1015 * start checking from it next time.
1016 */
1017 if (aggressive || !otherconn_checked_to)
1018 otherconn_checked_to = skb_check_prev;
1019 } else {
1020 otherconn_checked_to = aggressive ?
1021 skb_check : skb_check_prev;
1022 }
1023 }
1024 }
1025
1026 /* skb_check is reused at this point
1027 * it is the pure ACK to be filtered (if any)
1028 */
1029 skb_check = NULL;
1030
1031 /* next time start checking from the older/nearest to head of unfiltered
1032 * but important tcp packets from this connection and other connections.
1033 * if none seen, start after the last packet evaluated in the loop.
1034 */
1035 if (thisconn_checked_to && otherconn_checked_to)
1036 flow->ackcheck = thisconn_seen_last ?
1037 otherconn_checked_to : thisconn_checked_to;
1038 else if (thisconn_checked_to)
1039 flow->ackcheck = thisconn_checked_to;
1040 else if (otherconn_checked_to)
1041 flow->ackcheck = otherconn_checked_to;
1042 else
1043 flow->ackcheck = skb_check_prev;
1044
1045 /* if filtering, remove the pure ACK from the flow queue */
1046 if (thisconn_ack && (aggressive || thisconn_redundant_seen)) {
> 1047 if (PTR_ERR(thisconn_ack) == -1) {
1048 skb_check = flow->head;
1049 flow->head = flow->head->next;
1050 } else {
1051 skb_check = thisconn_ack->next;
1052 thisconn_ack->next = thisconn_ack->next->next;
1053 }
1054 }
1055
1056 /* we just filtered that ack, fix up the list */
1057 if (flow->ackcheck == skb_check)
1058 flow->ackcheck = thisconn_ack;
1059 /* check the entire flow queue next time */
1060 if (PTR_ERR(flow->ackcheck) == -1)
1061 flow->ackcheck = NULL;
1062
1063 return skb_check;
1064 }
1065
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Powered by blists - more mailing lists