| 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: <CAHqykcRn5vO+1sG28h-05Lgd-DRYHvszz-SLW38t8RToyhabLQ@mail.gmail.com>
Date: Tue, 27 Mar 2018 16:05:03 +0300
From: Noam Rathaus <noamr@...ondsecurity.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: netdev@...r.kernel.org
Subject: Re: [Patch net] llc: properly handle dev_queue_xmit() return value
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.wangcong@...il.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 <noamr@...ondsecurity.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.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)
Powered by blists - more mailing lists