netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarek Poplawski <jarkao2@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: f6bvp@free.fr, ralf@linux-mips.org, adobriyan@gmail.com,
	netdev@vger.kernel.org
Subject: Re: [PATCH][ROSE][AX25] af_ax25: possible circular locking
Date: Sun, 30 Dec 2007 15:13:23 +0100	[thread overview]
Message-ID: <20071230141323.GA3377@ami.dom.local> (raw)
In-Reply-To: <20071229.191443.10947724.davem@davemloft.net>

On Sat, Dec 29, 2007 at 07:14:43PM -0800, David Miller wrote:
...
> You can't just drop this linked list lock and expect it to stay
> consistent like that.
> 
> Once you drop it, any thread of control can get in there and delete
> entries from the list.
> 
> Since we know it can happen, using a WARN_ON_ONCE(1) is not
> appropriate.

The problem is 'we' don't know if it can happen... In the first
message with this patch I've tried to get this information, and
now it seems you are the only one with this knowledge, but of
course this is more than enough for me to agree with your decision
to dump this patch.

> [...]  And if it triggers it will do the wrong thing, because
> by branching back to "again" we can call ax25_disconnect() multiple
> times on the same entry which isn't right.

This is an equivalent of list_for_each_entry_safe(), and if such
WARN_ON is ever reported this would confirm the solution is wrong.
But, it seems Bernard's case was too simple to trigger this bug.
Alas it was complex enough to trigger this other bug...

"again" loop should skip this entry next time: s->ax25_dev should
be NULL, so not equal to ax25dev. (But of course there could be
this unknown to me place, which changes this back in the meantime.)

> You'll thus need to resolve this locking conflict more properly.
> I know it's hard, but your current fix is worse because it adds
> a new known bug.

Sorry, it seems this will've to wait until Ralf finds some time,
because I really can't give this more time, considering I never
used nor have any plans to use this code, and this bug could
suggest there is not so many interested in this, besides Bernard,
either.

Regards,
Jarek P.

  reply	other threads:[~2007-12-30 14:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-17 10:06 [ROSE] [AX25] possible circular locking Bernard Pidoux F6BVP
2007-12-18 13:52 ` Jarek Poplawski
     [not found]   ` <476837BF.3070207@free.fr>
2007-12-18 22:04     ` Jarek Poplawski
2007-12-28 21:30   ` Pidoux
     [not found]   ` <47755FDB.2070501@free.fr>
2007-12-28 21:48     ` [PATCH][ROSE][AX25] af_ax25: " Jarek Poplawski
2007-12-30  3:14       ` David Miller
2007-12-30 14:13         ` Jarek Poplawski [this message]
2007-12-31  5:00           ` David Miller
2008-01-11  5:22           ` David Miller
2008-01-11  9:40             ` Jarek Poplawski
2008-01-12 19:48               ` Bernard Pidoux F6BVP
2008-01-11 21:40             ` [PATCH] [ROSE] two extra tab characters removed Bernard Pidoux F6BVP
2008-02-09 18:44   ` [PATCH][AX25] ax25_ds_timer: use mod_timer instead of add_timer Bernard Pidoux F6BVP
2008-02-09 19:39     ` Jarek Poplawski
2008-02-10 18:07       ` Bernard Pidoux F6BVP
2008-02-09 23:50     ` [PATCH][AX25] af_ax25: remove sock lock in ax25_info_show() Jarek Poplawski
2008-02-10 13:10     ` [PATCH v2][AX25] " Jarek Poplawski
2008-02-12  5:25       ` David Miller

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=20071230141323.GA3377@ami.dom.local \
    --to=jarkao2@gmail.com \
    --cc=adobriyan@gmail.com \
    --cc=davem@davemloft.net \
    --cc=f6bvp@free.fr \
    --cc=netdev@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    /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 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).