linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: morpheus.ibis@gmail.com (Pavel Herrmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] MAX1111: Fix Race condition causing NULL pointer exception
Date: Mon, 11 Jul 2011 22:36:31 +0200	[thread overview]
Message-ID: <1602583.Pi9fQvRk5m@bloomfield> (raw)
In-Reply-To: <20110711221148.3a76d55b@endymion.delvare>

On Monday 11 of July 2011 22:11:48 Jean Delvare wrote:
> > spi_sync call uses its spi_message parameter to keep completion
> > information, having this structure static is not thread-safe,
> > potentially causing one thread having pointers to memory on or above
> > other threads stack. use mutex to prevent multiple access
> 
> This has nothing to do with static, as a matter of fact the structure
> is dynamically allocated. The bottom line is that the driver structure
> is such that calls to max1111_read() must be serialized.

the structure is dynamically allocated, but the pointer used to hold it is a 
static global var.
"static" in this context meant "shared by all threads"

> > +	/* spi_sync requires data not to be freed before function returns
> > +	 * for static data, any access is dangerous, use locks
> > +	 */
> 
> This has nothing to do with "freeing data". max1111_read() doesn't free
> anything. It is making use of a data structure, the access to which
> must be serialized. Easy as that. And no, access isn't dangerous ;)

as spi_message contains a pointer to completion (created and waited on by 
spi_sync()), witch gets rewritten and causes the NULL exception, writing to it 
while the call is in progress is bad idea. also changing the message sent 
half-way would not be very nice.
reading would be fine, though

> Please respin your patch with a better struct member name and improved
> description and comments, and I'll be happy to apply it.

on it

  reply	other threads:[~2011-07-11 20:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-11 18:47 [PATCH v2] MAX1111: Fix Race condition causing NULL pointer exception Pavel Herrmann
2011-07-11 20:11 ` Jean Delvare
2011-07-11 20:36   ` Pavel Herrmann [this message]
2011-07-11 21:03     ` Guenter Roeck
2011-07-11 21:49       ` Pavel Herrmann
2011-07-12  6:48         ` Jean Delvare
2011-07-11 20:56 ` [Zaurus-devel] " Stanislav Brabec
  -- strict thread matches above, loose matches on Subject: below --
2011-06-03 20:00 Pavel Herrmann
2011-06-29 20:11 ` Pavel Herrmann

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=1602583.Pi9fQvRk5m@bloomfield \
    --to=morpheus.ibis@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.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).