Skip to content

Commit

Permalink
llc: properly handle dev_queue_xmit() return value
Browse files Browse the repository at this point in the history 
llc_conn_send_pdu() pushes the skb into write queue and
calls llc_conn_send_pdus() to flush them out. However, the
status of dev_queue_xmit() is not returned to caller,
in this case, llc_conn_state_process().

llc_conn_state_process() needs hold the skb no matter
success or failure, because it still uses it after that,
therefore we should hold skb before dev_queue_xmit() when
that skb is the one being processed by llc_conn_state_process().

For other callers, they can just pass NULL and ignore
the return value as they are.

Reported-by: Noam Rathaus <noamr@beyondsecurity.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Cong Wang authored and David S. Miller committed Mar 27, 2018
1 parent 2a7fdec commit b85ab56
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 16 deletions.
2 changes: 1 addition & 1 deletion include/net/llc_conn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ void llc_sk_reset(struct sock *sk);

/* Access to a connection */
int llc_conn_state_process(struct sock *sk, struct sk_buff *skb);
void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_rtn_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit);
void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit);
Expand Down
15 changes: 9 additions & 6 deletions net/llc/llc_c_ac.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ static int llc_conn_ac_send_i_cmd_p_set_0(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
llc_conn_send_pdu(sk, skb);
rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
Expand Down Expand Up @@ -916,7 +916,7 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
llc_pdu_init_as_i_cmd(skb, llc->ack_pf, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
llc_conn_send_pdu(sk, skb);
rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
Expand All @@ -935,14 +935,17 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
int llc_conn_ac_send_i_as_ack(struct sock *sk, struct sk_buff *skb)
{
struct llc_sock *llc = llc_sk(sk);
int ret;

if (llc->ack_must_be_send) {
llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
ret = llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
llc->ack_must_be_send = 0 ;
llc->ack_pf = 0;
} else
llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
return 0;
} else {
ret = llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
}

return ret;
}

/**
Expand Down
32 changes: 23 additions & 9 deletions net/llc/llc_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#endif

static int llc_find_offset(int state, int ev_type);
static void llc_conn_send_pdus(struct sock *sk);
static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *skb);
static int llc_conn_service(struct sock *sk, struct sk_buff *skb);
static int llc_exec_conn_trans_actions(struct sock *sk,
struct llc_conn_state_trans *trans,
Expand Down Expand Up @@ -193,11 +193,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
return rc;
}

void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
{
/* queue PDU to send to MAC layer */
skb_queue_tail(&sk->sk_write_queue, skb);
llc_conn_send_pdus(sk);
return llc_conn_send_pdus(sk, skb);
}

/**
Expand Down Expand Up @@ -255,7 +255,7 @@ void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
llc_conn_send_pdus(sk);
llc_conn_send_pdus(sk, NULL);
out:;
}

Expand Down Expand Up @@ -296,7 +296,7 @@ void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
llc_conn_send_pdus(sk);
llc_conn_send_pdus(sk, NULL);
out:;
}

Expand Down Expand Up @@ -340,12 +340,16 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked)
/**
* llc_conn_send_pdus - Sends queued PDUs
* @sk: active connection
* @hold_skb: the skb held by caller, or NULL if does not care
*
* Sends queued pdus to MAC layer for transmission.
* Sends queued pdus to MAC layer for transmission. When @hold_skb is
* NULL, always return 0. Otherwise, return 0 if @hold_skb is sent
* successfully, or 1 for failure.
*/
static void llc_conn_send_pdus(struct sock *sk)
static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
{
struct sk_buff *skb;
int ret = 0;

while ((skb = skb_dequeue(&sk->sk_write_queue)) != NULL) {
struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
Expand All @@ -357,10 +361,20 @@ static void llc_conn_send_pdus(struct sock *sk)
skb_queue_tail(&llc_sk(sk)->pdu_unack_q, skb);
if (!skb2)
break;
skb = skb2;
dev_queue_xmit(skb2);
} else {
bool is_target = skb == hold_skb;
int rc;

if (is_target)
skb_get(skb);
rc = dev_queue_xmit(skb);
if (is_target)
ret = rc;
}
dev_queue_xmit(skb);
}

return ret;
}

/**
Expand Down

0 comments on commit b85ab56

Please sign in to comment.