Hi, I am not sure what is the next step from this?
Does it mean that a patch is out in the kernel's GIT/Beta version? Or is this just a proposal? On Tue, Mar 27, 2018 at 1:08 AM, Cong Wang <xiyou.wangc...@gmail.com> wrote: > 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 <no...@beyondsecurity.com> > Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com> > --- > include/net/llc_conn.h | 2 +- > net/llc/llc_c_ac.c | 15 +++++++++------ > net/llc/llc_conn.c | 32 +++++++++++++++++++++++--------- > 3 files changed, 33 insertions(+), 16 deletions(-) > > diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h > index fe994d2e5286..5c40f118c0fa 100644 > --- a/include/net/llc_conn.h > +++ b/include/net/llc_conn.h > @@ -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); > diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c > index f59648018060..163121192aca 100644 > --- a/net/llc/llc_c_ac.c > +++ b/net/llc/llc_c_ac.c > @@ -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; > @@ -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; > @@ -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; > } > > /** > diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c > index 9177dbb16dce..110e32bcb399 100644 > --- a/net/llc/llc_conn.c > +++ b/net/llc/llc_conn.c > @@ -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, > @@ -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); > } > > /** > @@ -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:; > } > > @@ -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:; > } > > @@ -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); > @@ -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; > } > > /** > -- > 2.13.0 > -- Thanks, Noam Rathaus Beyond Security PGP Key ID: 7EF920D3C045D63F (Exp 2019-03)