Re: [PATCH v1 10/40] j1939: socket: make sure all sessions are finished on close

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Robin,

On 08.01.19 11:26, Robin van der Gracht wrote:
On Thu, 13 Dec 2018 16:31:27 +0100
Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:

Currently, (E)TP transfers will be aborted, as soon application will
call close() or exit, as the socket will be automatically closed by the
kernel.

Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
---
  net/can/j1939/socket.c | 10 ++++++++++
  1 file changed, 10 insertions(+)

diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index d6332483350f..e003d3cae24c 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -101,6 +101,13 @@ static inline void j1939_sock_pending_add(struct sock *sk)
  	atomic_inc(&jsk->skb_pending);
  }
+static int j1939_sock_pending_get(struct sock *sk)
+{
+	struct j1939_sock *jsk = j1939_sk(sk);
+
+	return atomic_read(&jsk->skb_pending);
+}
+
  void j1939_sock_pending_del(struct sock *sk)
  {
  	struct j1939_sock *jsk = j1939_sk(sk);
@@ -404,6 +411,9 @@ static int j1939_sk_release(struct socket *sock)
  	if (jsk->state & J1939_SOCK_BOUND) {
  		struct net_device *ndev;
+ wait_event_interruptible(jsk->waitq,
+					 j1939_sock_pending_get(&jsk->sk) == 0);
+
  		spin_lock_bh(&j1939_socks_lock);
  		list_del_init(&jsk->list);
  		spin_unlock_bh(&j1939_socks_lock);

close() now blocks if data was successfully send on that socket.
This is probably due to a resource issue which is now brought to light.

Seems as if jsk->skb_pending is incremented on every j1939_sk_sendmsg()
call, but only decremented when an error occurs. So when a send()
succeeds, close() will block indefinitely.


It is decremented in:
 static void __j1939_session_drop(struct j1939_session *session)
 {
        struct j1939_priv *priv = session->priv;
-       struct sk_buff *se_skb = j1939_session_skb_find(session);

-       if (session->transmission) {
-               if (se_skb && se_skb->sk)
-                       j1939_sock_pending_del(se_skb->sk);
-               wake_up_all(&priv->tp_wait);
-       }
+       if (!session->transmission)
+               return;
+
+       j1939_sock_pending_del(session->sk);
+       wake_up_all(&priv->tp_wait);
 }

Is it a theoretical or real issue?



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux