All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Fulghum <paulkf@microgate.com>
To: Zhengchao Shao <shaozhengchao@huawei.com>
Cc: "linux-kernel@vger.kernel.org Mailing List" 
	<linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	akpm@linux-foundation.org, weiyongjun1@huawei.com,
	yuehaibing@huawei.com
Subject: [PATCH] tty: synclink_gt: unwind actions in error path of net device open
Date: Tue, 15 Nov 2022 09:18:41 -0800	[thread overview]
Message-ID: <D2A8DDC3-9D72-4CDD-92A0-2C27146C1BE0@microgate.com> (raw)
In-Reply-To: <387F9F7B-C3AF-45BF-94ED-59348990B407@microgate.com>

Resent in plain text, sorry for the previous HTML.

Zhengchao Shao <shaozhengchao@huawei.com> identified by inspection bugs in the error path of hdlcdev_open() in synclink_gt.c

The function did not fully unwind actions in the error path. The use of try_module_get()/module_put() is unnecessary, potentially hazardous and is removed. The synclink_gt driver is already pinned any point the net device is registered, a requirement for calling this entry point.

The call hdlc_open() to init the generic HDLC layer is moved to after driver level init/checks and proper rollback of previous actions is added. This is a more sensible ordering as the most common error paths are at the driver level and the driver level rollbacks require less processing than hdlc_open()/hdlc_close().

This has been tested with supported hardware.

Signed-off-by:Paul Fulghum <paulkf@microgate.com>

diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 25e9befdda3a..72b76cdde534 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -1433,16 +1433,8 @@ static int hdlcdev_open(struct net_device *dev)
 	int rc;
 	unsigned long flags;
 
-	if (!try_module_get(THIS_MODULE))
-		return -EBUSY;
-
 	DBGINFO(("%s hdlcdev_open\n", dev->name));
 
-	/* generic HDLC layer open processing */
-	rc = hdlc_open(dev);
-	if (rc)
-		return rc;
-
 	/* arbitrate between network and tty opens */
 	spin_lock_irqsave(&info->netlock, flags);
 	if (info->port.count != 0 || info->netcount != 0) {
@@ -1461,6 +1453,16 @@ static int hdlcdev_open(struct net_device *dev)
 		return rc;
 	}
 
+	/* generic HDLC layer open processing */
+	rc = hdlc_open(dev);
+	if (rc) {
+		shutdown(info);
+		spin_lock_irqsave(&info->netlock, flags);
+		info->netcount = 0;
+		spin_unlock_irqrestore(&info->netlock, flags);
+		return rc;
+	}
+
 	/* assert RTS and DTR, apply hardware settings */
 	info->signals |= SerialSignal_RTS | SerialSignal_DTR;
 	program_hw(info);
@@ -1506,7 +1508,6 @@ static int hdlcdev_close(struct net_device *dev)
 	info->netcount=0;
 	spin_unlock_irqrestore(&info->netlock, flags);
 
-	module_put(THIS_MODULE);
 	return 0;
 }
 


  parent reply	other threads:[~2022-11-15 17:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14  1:07 [PATCH] tty: synclink_gt: release resources when synclink_gt driver open failed Zhengchao Shao
2022-11-14  5:44 ` Jiri Slaby
2022-11-14  7:19 ` Greg KH
2022-11-14 16:47 ` Paul Fulghum
     [not found]   ` <387F9F7B-C3AF-45BF-94ED-59348990B407@microgate.com>
2022-11-15 17:10     ` [PATCH] tty: synclink_gt: unwind actions in error path of net device open Greg Kroah-Hartman
2022-11-15 17:38       ` Paul Fulghum
2022-11-15 17:54         ` Greg Kroah-Hartman
2022-11-15 17:18     ` Paul Fulghum [this message]
2022-11-15 18:21 ` Paul Fulghum
2022-11-15 18:48   ` Greg KH
2022-11-18  3:09 ` [PATCH v2] tty:synclink_gt unwind actions in error path Paul Fulghum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D2A8DDC3-9D72-4CDD-92A0-2C27146C1BE0@microgate.com \
    --to=paulkf@microgate.com \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shaozhengchao@huawei.com \
    --cc=weiyongjun1@huawei.com \
    --cc=yuehaibing@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.