linux-ppp.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: possible deadlock in ppp_dev_uninit
       [not found] <001a11c006da57e7ff0561edda2b@google.com>
@ 2018-01-05 18:15 ` Guillaume Nault
  2018-01-05 18:27   ` Guillaume Nault
  0 siblings, 1 reply; 2+ messages in thread
From: Guillaume Nault @ 2018-01-05 18:15 UTC (permalink / raw)
  To: syzbot; +Cc: linux-kernel, linux-ppp, netdev, paulus, syzkaller-bugs

On Wed, Jan 03, 2018 at 10:58:01PM -0800, syzbot wrote:
> Hello,
> 
> ======================
> WARNING: possible recursive locking detected
> 4.15.0-rc6-next-20180103+ #87 Not tainted
> --------------------------------------------
> syzkaller221540/3462 is trying to acquire lock:
>  (&pn->all_ppp_mutex){+.+.}, at: [<00000000709ea4fe>]
> ppp_dev_uninit+0x1be/0x390 drivers/net/ppp/ppp_generic.c:1369
> 
> but task is already holding lock:
>  (&pn->all_ppp_mutex){+.+.}, at: [<00000000752caad5>] ppp_unit_register
> drivers/net/ppp/ppp_generic.c:981 [inline]
>  (&pn->all_ppp_mutex){+.+.}, at: [<00000000752caad5>]
> ppp_dev_configure+0x6a4/0xc40 drivers/net/ppp/ppp_generic.c:1066
> 
ppp_unit_register() acquires pn->all_ppp_mutex while calling
register_netdevice(). If register_netdevice() fails, it can call
ppp_dev_uninit() which then tries to lock pn->all_ppp_mutex again.

Maybe unlocking pn->all_ppp_mutex before register_netdevice() would be
enough, but that'd make the unit visible while the PPP device isn't yet
registered. I'm going to check if that can be a problem.

That's probably worth a test anyway.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master

-------- 8< --------

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index d8e5747ff4e3..264d4af0bf69 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -1006,17 +1006,18 @@ static int ppp_unit_register(struct ppp *ppp, int unit, bool ifname_is_set)
        if (!ifname_is_set)
                snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index);
 
+       mutex_unlock(&pn->all_ppp_mutex);
+
        ret = register_netdevice(ppp->dev);
        if (ret < 0)
                goto err_unit;
 
        atomic_inc(&ppp_unit_count);
 
-       mutex_unlock(&pn->all_ppp_mutex);
-
        return 0;
 
 err_unit:
+       mutex_lock(&pn->all_ppp_mutex);
        unit_put(&pn->units_idr, ppp->file.index);
 err:
        mutex_unlock(&pn->all_ppp_mutex);


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: possible deadlock in ppp_dev_uninit
  2018-01-05 18:15 ` possible deadlock in ppp_dev_uninit Guillaume Nault
@ 2018-01-05 18:27   ` Guillaume Nault
  0 siblings, 0 replies; 2+ messages in thread
From: Guillaume Nault @ 2018-01-05 18:27 UTC (permalink / raw)
  To: syzbot; +Cc: linux-kernel, linux-ppp, netdev, paulus, syzkaller-bugs

On Fri, Jan 05, 2018 at 07:15:31PM +0100, Guillaume Nault wrote:
> That's probably worth a test anyway.
>
Copy/paste error :-/

Here's a version that should apply cleanly.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master

-------- 8< --------

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index d8e5747ff4e3..264d4af0bf69 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -1006,17 +1006,18 @@ static int ppp_unit_register(struct ppp *ppp, int unit, bool ifname_is_set)
 	if (!ifname_is_set)
 		snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index);
 
+	mutex_unlock(&pn->all_ppp_mutex);
+
 	ret = register_netdevice(ppp->dev);
 	if (ret < 0)
 		goto err_unit;
 
 	atomic_inc(&ppp_unit_count);
 
-	mutex_unlock(&pn->all_ppp_mutex);
-
 	return 0;
 
 err_unit:
+	mutex_lock(&pn->all_ppp_mutex);
 	unit_put(&pn->units_idr, ppp->file.index);
 err:
 	mutex_unlock(&pn->all_ppp_mutex);

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-01-05 18:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <001a11c006da57e7ff0561edda2b@google.com>
2018-01-05 18:15 ` possible deadlock in ppp_dev_uninit Guillaume Nault
2018-01-05 18:27   ` Guillaume Nault

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).