Fix refcount leak in tty_port.c

Hello,

In linux 3.10 in the file drivers/tty/tty_port.c the function tty_port_tty_hangup may leak a tty reference:

        struct tty_struct *tty = tty_port_tty_get(port);

        if (tty && (!check_clocal || !C_CLOCAL(tty))) {
                tty_hangup(tty);
                tty_kref_put(tty);
        }

If tty != NULL and the second condition is false we never call tty_kref_put and the reference is leaked.

Fix by nesting two if statements.

Signed-off-by Gianluca Anzolin <[email protected]>

Hi Gianluca,

Quoted 14 lines · Gianluca Anzolin

Hello,

In linux 3.10 in the file drivers/tty/tty_port.c the function tty_port_tty_hangup may leak a tty reference:

         struct tty_struct *tty = tty_port_tty_get(port);

         if (tty && (!check_clocal || !C_CLOCAL(tty))) {
                 tty_hangup(tty);
                 tty_kref_put(tty);
         }

If tty != NULL and the second condition is false we never call tty_kref_put and the reference is leaked.

Quoted 3 lines

Fix by nesting two if statements.

Signed-off-by Gianluca Anzolin <[email protected]>

As mentioned by Gianluca this is a regression of aa27a094 and we depend on this patch to go ahead with some fixes in the bluetooth subsystem.

Gianluca, it might help if you send a proper git inline formated patch, mentioning the issue and which regression you are fixing. It makes maintainer's life easier.

Also add my Ack to the patch:

Acked-by Gustavo Padovan <[email protected]>

Gustavo

drivers/tty/tty_port.c +0 -0

index 121aeb9..2198f7d 100644
@@ -256,8 +256,9 @@ void tty_port_tty_hangup(struct tty_port *port, bool check_clocal)
 {

Quoted 18 lines · Gustavo Padovan

Hi Gianluca,

Quoted 14 lines · Gianluca Anzolin

Hello,

In linux 3.10 in the file drivers/tty/tty_port.c the function tty_port_tty_hangup may leak a tty reference:

         struct tty_struct *tty = tty_port_tty_get(port);

         if (tty && (!check_clocal || !C_CLOCAL(tty))) {
                 tty_hangup(tty);
                 tty_kref_put(tty);
         }

If tty != NULL and the second condition is false we never call tty_kref_put and the reference is leaked.

Good catch.

Quoted 6 lines

Quoted 3 lines

Fix by nesting two if statements.

Signed-off-by Gianluca Anzolin <[email protected]>

As mentioned by Gianluca this is a regression of aa27a094 and we depend on this patch to go ahead with some fixes in the bluetooth subsystem.

Gustavo,

There's no direct dependency; ie., there aren't merge issues here. We should progress with the fixes to rfcomm independent of this patch.

Quoted 3 lines

Gianluca, it might help if you send a proper git inline formated patch, mentioning the issue and which regression you are fixing. It makes maintainer's life easier.

As Gustavo points out, please inline the patch otherwise commenters have to do it for you.

Quoted 6 lines

Also add my Ack to the patch:

Acked-by Gustavo Padovan <[email protected]>

Gustavo

Copy of the Gianluca's patch with my comments

--- %< ---

Please put a proper commit message here, including that this is a regression and the commit id that caused the regression so this patch can eventually make its way to stable.

Quoted 6 lines

drivers/tty/tty_port.c +0 -0

index 121aeb9..2198f7d 100644
@@ -256,8 +256,9 @@ void tty_port_tty_hangup(struct tty_port *port, bool check_clocal)
 {

drivers/tty/tty_port.c void tty_port_tty_hangup(struct tty_port *port, bool check_clocal) · line 264

Quoted 10 lines

drivers/tty/tty_port.c +3 -2

     struct tty_struct *tty = tty_port_tty_get(port);
 
-    if (tty && (!check_clocal || !C_CLOCAL(tty))) {
-        tty_hangup(tty);
+    if (tty) {
+        if (!check_clocal || !C_CLOCAL(tty))
+            tty_hangup(tty);
         tty_kref_put(tty);
     }
 }

tty_kref_put() already checks for NULL tty. I would prefer:

{
       struct tty_struct *tty = tty_port_tty_get(port);

     if (tty && (!check_clocal || !C_CLOCAL(tty)))
         tty_hangup(tty);
     tty_kref_put(tty);
}

Quoted 34 lines · Peter Hurley

Quoted 18 lines · Gustavo Padovan

Hi Gianluca,

Quoted 14 lines · Gianluca Anzolin

Hello,

In linux 3.10 in the file drivers/tty/tty_port.c the function tty_port_tty_hangup may leak a tty reference:

        struct tty_struct *tty = tty_port_tty_get(port);

        if (tty && (!check_clocal || !C_CLOCAL(tty))) {
                tty_hangup(tty);
                tty_kref_put(tty);
        }

If tty != NULL and the second condition is false we never call tty_kref_put and the reference is leaked.

Good catch.

Quoted 6 lines

Quoted 3 lines

Fix by nesting two if statements.

Signed-off-by Gianluca Anzolin <[email protected]>

As mentioned by Gianluca this is a regression of aa27a094 and we depend on this patch to go ahead with some fixes in the bluetooth subsystem.

Gustavo,

There's no direct dependency; ie., there aren't merge issues here. We should progress with the fixes to rfcomm independent of this patch.

Quoted 6 lines

Quoted 3 lines

Gianluca, it might help if you send a proper git inline formated patch, mentioning the issue and which regression you are fixing. It makes maintainer's life easier.

As Gustavo points out, please inline the patch otherwise commenters have to do it for you.

Gianluca,

I think Greg may be expecting you to address the comments from myself and Gustavo before accepting this patch.

Greg, is that the case?