linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Greer <mgreer@animalcreek.com>
To: Justin Bronder <justin@kuvee.com>
Cc: Geoff Lansberry <geoff@kuvee.com>,
	linux-wireless@vger.kernel.org, lauro.venancio@openbossa.org,
	aloisio.almeida@openbossa.org, sameo@linux.intel.com,
	robh+dt@kernel.org, mark.rutland@arm.com, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jaret Cantu <jaret.cantu@timesys.com>
Subject: Re: nfc: trf7970a: Prevent repeated polling from crashing the kernel
Date: Tue, 20 Dec 2016 12:56:21 -0700	[thread overview]
Message-ID: <20161220195621.GA6400@animalcreek.com> (raw)
In-Reply-To: <20161220191352.GB23496@lasswell.members.linode.com>

On Tue, Dec 20, 2016 at 02:13:52PM -0500, Justin Bronder wrote:
> On 20/12/16 11:59 -0700, Mark Greer wrote:
> > On Tue, Dec 20, 2016 at 11:16:32AM -0500, Geoff Lansberry wrote:
> > > From: Jaret Cantu <jaret.cantu@timesys.com>
> > > 
> > > Repeated polling attempts cause a NULL dereference error to occur.
> > > This is because the state of the trf7970a is currently reading but
> > > another request has been made to send a command before it has finished.
> > 
> > How is this happening?  Was trf7970a_abort_cmd() called and it didn't
> > work right?  Was it not called at all and there is a bug in the digital
> > layer?  More details please.
> > 
> > > The solution is to properly kill the waiting reading (workqueue)
> > > before failing on the send.
> > 
> > If the bug is in the calling code, then that is what should get fixed.
> > This seems to be a hack to work-around a digital layer bug.
> 
> One of our uses of NFC is to begin polling to read a tag and then stop polling
> (in order to save power) until we know via user interaction that we need to poll
> again.  This is typically many minutes later so the power saving is pretty
> significant.  However, it's possible that a user will remove the tag before
> reading has completed.  We also detect this case and stop polling.  I can go
> more into this if necessary but that is what exposed a panic.
> 
> You can reproduce using neard and python, in our testing it was very likely to
> occur in 10-100 iterations of the following.:
> 
>     #!/usr/bin/python
>     import time
> 
>     import dbus
> 
>     bus = dbus.SystemBus()
>     nfc0 = bus.get_object('org.neard', '/org/neard/nfc0')
>     props = dbus.Interface(nfc0, 'org.freedesktop.DBus.Properties')
> 
>     try:
>         props.Set('org.neard.Adapter', 'Powered', dbus.Boolean(1))
>     except:
>         pass
> 
>     adapter = dbus.Interface(nfc0, 'org.neard.Adapter')
> 
>     for i in range(1000):
>         adapter.StartPollLoop('Initiator')
>         time.sleep(0.1)
>         adapter.StopPollLoop()
>         print(i)
> 
> I believe the last time we tested this was around the 4.1 release.

Thanks for the info, Justin, but I was also seeking more information
at the kernel NFC subsystem and trf7970a driver level.  This patch
adds code inside an 'if' in the driver whose condition should never
be evaluate to true but apparently it did.  How?

Thanks,

Mark
--

  reply	other threads:[~2016-12-20 19:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-20 16:16 [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock Geoff Lansberry
2016-12-20 16:16 ` [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Geoff Lansberry
2016-12-21  2:23   ` Mark Greer
2016-12-21 11:47     ` Geoff Lansberry
2016-12-21 16:13       ` Mark Greer
2016-12-20 16:16 ` [PATCH 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel Geoff Lansberry
2016-12-20 18:59   ` Mark Greer
2016-12-20 19:13     ` Justin Bronder
2016-12-20 19:56       ` Mark Greer [this message]
2016-12-20 17:58 ` [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock Jones Desougi
2016-12-20 18:11 ` Mark Greer
2016-12-20 18:29   ` Geoff Lansberry
2016-12-20 18:35     ` Mark Greer

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=20161220195621.GA6400@animalcreek.com \
    --to=mgreer@animalcreek.com \
    --cc=aloisio.almeida@openbossa.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geoff@kuvee.com \
    --cc=jaret.cantu@timesys.com \
    --cc=justin@kuvee.com \
    --cc=lauro.venancio@openbossa.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=sameo@linux.intel.com \
    --subject='Re: nfc: trf7970a: Prevent repeated polling from crashing the kernel' \
    /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

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).