From 7e0309631ecf0cd16edba72ff74747fa1b96ead3 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Wed, 1 Jan 2014 23:04:25 +0100 Subject: net: llc: fix order of evaluation in llc_conn_ac_inc_vr_by_1 Function llc_conn_ac_inc_vr_by_1() evaluates via macro PDU_GET_NEXT_Vr() into ... llc_sk(sk)->vR = ++llc_sk(sk)->vR & 0xffffffffffffff7f ... but the order in which the side effects take place is undefined because there is no intervening sequence point. As llc_sk(sk)->vR is written in llc_sk(sk)->vR (assignment left-hand side) and written in ++llc_sk(sk)->vR & 0xffffffffffffff7f this might possibly yield undefined behavior. The final value of llc_sk(sk)->vR is ambiguous, because, depending on the order of expression evaluation, the increment may occur before, after, or interleaved with the assignment. In C, evaluating such an expression yields undefined behavior. Since we're doing the increment via PDU_GET_NEXT_Vr() macro and the only place it is being used is from llc_conn_ac_inc_vr_by_1(), in order to increment vR by 1 with a follow-up optimized modulo, rewrite the expression into ((vR + 1) & CONST) in order to fix this. Signed-off-by: Daniel Borkmann Cc: Arnaldo Carvalho de Melo Cc: Stephen Hemminger Signed-off-by: David S. Miller --- include/net/llc_pdu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/net') diff --git a/include/net/llc_pdu.h b/include/net/llc_pdu.h index 31e2de7..c0f0a13 100644 --- a/include/net/llc_pdu.h +++ b/include/net/llc_pdu.h @@ -142,7 +142,7 @@ #define LLC_S_PF_IS_1(pdu) ((pdu->ctrl_2 & LLC_S_PF_BIT_MASK) ? 1 : 0) #define PDU_SUPV_GET_Nr(pdu) ((pdu->ctrl_2 & 0xFE) >> 1) -#define PDU_GET_NEXT_Vr(sn) (++sn & ~LLC_2_SEQ_NBR_MODULO) +#define PDU_GET_NEXT_Vr(sn) (((sn) + 1) & ~LLC_2_SEQ_NBR_MODULO) /* FRMR information field macros */ -- cgit v1.1 From 619a60ee04be33238721a15c1f9704a2a515a33e Mon Sep 17 00:00:00 2001 From: Vlad Yasevich Date: Thu, 2 Jan 2014 14:39:44 -0500 Subject: sctp: Remove outqueue empty state The SCTP outqueue structure maintains a data chunks that are pending transmission, the list of chunks that are pending a retransmission and a length of data in flight. It also tries to keep the emtpy state so that it can performe shutdown sequence or notify user. The problem is that the empy state is inconsistently tracked. It is possible to completely drain the queue without sending anything when using PR-SCTP. In this case, the empty state will not be correctly state as report by Jamal Hadi Salim . This can cause an association to be perminantly stuck in the SHUTDOWN_PENDING state. Additionally, SCTP is incredibly inefficient when setting the empty state. Even though all the data is availaible in the outqueue structure, we ignore it and walk a list of trasnports. In the end, we can completely remove the extra empty state and figure out if the queue is empty by looking at 3 things: length of pending data, length of in-flight data, and exisiting of retransmit data. All of these are already in the strucutre. Reported-by: Jamal Hadi Salim Signed-off-by: Vlad Yasevich Acked-by: Neil Horman Tested-by: Jamal Hadi Salim Signed-off-by: David S. Miller --- include/net/sctp/structs.h | 3 --- 1 file changed, 3 deletions(-) (limited to 'include/net') diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 67b5d00..0a248b3 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -1046,9 +1046,6 @@ struct sctp_outq { /* Corked? */ char cork; - - /* Is this structure empty? */ - char empty; }; void sctp_outq_init(struct sctp_association *, struct sctp_outq *); -- cgit v1.1