Re: [Patch net] llc: properly handle dev_queue_xmit() return value

[Noam Rathaus]

Hi,

Is there any update?

On Fri, Apr 13, 2018 at 7:49 PM, Noam Rathaus  wrote:
> Hi
>
> Any update?
>
> On Thu, 29 Mar 2018 at 14:11, Noam Rathaus  wrote:
>>
>> Hi,
>>
>> Will you notify me when its been accepted? if not, how can I do this
>> checking myself to see if it was accepted?
>>
>> On Tue, Mar 27, 2018 at 8:13 PM, David Miller  wrote:
>> > From: Noam Rathaus 
>> > Date: Tue, 27 Mar 2018 16:27:49 +
>> >
>> >> Guys please fill me in on the next step?
>> >>
>> >> If it’s applied it means it’s part of the official code of the kernel
>> >> now?
>> >
>> > It means it is in my networking GIT tree and will make it's way to Linus
>> > in the not so distant future.
>>
>>
>>
>> --
>>
>> Thanks,
>> Noam Rathaus
>> Beyond Security
>>
>> PGP Key ID: 7EF920D3C045D63F (Exp 2019-03)
>
> --
> Thanks,
> Noam Rathaus



-- 

Thanks,
Noam Rathaus
Beyond Security

PGP Key ID: 7EF920D3C045D63F (Exp 2019-03)

Re: [Patch net] llc: properly handle dev_queue_xmit() return value

[Noam Rathaus]

Hi,

Will you notify me when its been accepted? if not, how can I do this
checking myself to see if it was accepted?

On Tue, Mar 27, 2018 at 8:13 PM, David Miller  wrote:
> From: Noam Rathaus 
> Date: Tue, 27 Mar 2018 16:27:49 +
>
>> Guys please fill me in on the next step?
>>
>> If it’s applied it means it’s part of the official code of the kernel now?
>
> It means it is in my networking GIT tree and will make it's way to Linus
> in the not so distant future.



-- 

Thanks,
Noam Rathaus
Beyond Security

PGP Key ID: 7EF920D3C045D63F (Exp 2019-03)

Re: [Patch net] llc: properly handle dev_queue_xmit() return value

[David Miller]

From: Noam Rathaus 
Date: Tue, 27 Mar 2018 16:27:49 +

> Guys please fill me in on the next step?
> 
> If it’s applied it means it’s part of the official code of the kernel now?

It means it is in my networking GIT tree and will make it's way to Linus
in the not so distant future.

Re: [Patch net] llc: properly handle dev_queue_xmit() return value

[David Miller]

From: Cong Wang 
Date: Mon, 26 Mar 2018 15:08:33 -0700

> 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 
> Signed-off-by: Cong Wang 

Applied, thanks Cong.

Re: [Patch net] llc: properly handle dev_queue_xmit() return value

[Noam Rathaus]

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  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 
> Signed-off-by: Cong Wang 
> ---
>  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_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 

[Patch net] llc: properly handle dev_queue_xmit() return value

[Cong Wang]

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 
Signed-off-by: Cong Wang 
---
 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_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
+ *