From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Greg KH <gregkh@linuxfoundation.org>, Cyril Bur <cyrilbur@gmail.com>
Cc: devicetree@vger.kernel.org, jassisinghbrar@gmail.com,
arnd@arndb.de, joel@jms.id.au, mark.rutland@arm.com,
robh+dt@kernel.org, openbmc@lists.ozlabs.org, andrew@aj.id.au,
xow@google.com, jk@ozlabs.org
Subject: Re: [PATCH 3/4] drivers/misc: Add ASpeed LPC control driver
Date: Thu, 12 Jan 2017 09:27:47 -0600 [thread overview]
Message-ID: <1484234867.2492.39.camel@kernel.crashing.org> (raw)
In-Reply-To: <20170112103038.GA19239@kroah.com>
On Thu, 2017-01-12 at 11:30 +0100, Greg KH wrote:
> > > And don't call access_ok(), it's racy and no driver should ever do that.
> > >
> >
> > Oh, duly noted. I'll be sure to check out how and why. Perhaps it
> > would be wise that no driver actually do that, I'm quite sure I used
> > other drivers as examples of best practice.
>
> You did? Please point me at that code so we can fix them up properly.
> Cargo-cult coding is not a good thing, but it happens, so if we can at
> least provide clean code to fixate on, it's good overall for everyone.
How so ? I mean, access_ok followed by __get/__put_user is still a
classic, what's wrong with it ?
The test performed by access_ok tests whether the address hits the
kernel/userspace limit, that limit doesn't change afaik, so there is no
race there, and avoids repeatedly testing it for series of subsequent
__get_user or __put_user.
What's wrong with them ?
Cheers,
Ben.
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
To: Greg KH
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
arnd-r2nGTMty4D4@public.gmane.org,
joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
andrew-zrmu5oMJ5Fs@public.gmane.org,
xow-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
Subject: Re: [PATCH 3/4] drivers/misc: Add ASpeed LPC control driver
Date: Thu, 12 Jan 2017 09:27:47 -0600 [thread overview]
Message-ID: <1484234867.2492.39.camel@kernel.crashing.org> (raw)
In-Reply-To: <20170112103038.GA19239-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Thu, 2017-01-12 at 11:30 +0100, Greg KH wrote:
> > > And don't call access_ok(), it's racy and no driver should ever do that.
> > >
> >
> > Oh, duly noted. I'll be sure to check out how and why. Perhaps it
> > would be wise that no driver actually do that, I'm quite sure I used
> > other drivers as examples of best practice.
>
> You did? Please point me at that code so we can fix them up properly.
> Cargo-cult coding is not a good thing, but it happens, so if we can at
> least provide clean code to fixate on, it's good overall for everyone.
How so ? I mean, access_ok followed by __get/__put_user is still a
classic, what's wrong with it ?
The test performed by access_ok tests whether the address hits the
kernel/userspace limit, that limit doesn't change afaik, so there is no
race there, and avoids repeatedly testing it for series of subsequent
__get_user or __put_user.
What's wrong with them ?
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-01-12 15:28 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-12 0:29 [PATCH 0/4] ASpeed mailbox and LPC control drivers Cyril Bur
2017-01-12 0:29 ` Cyril Bur
2017-01-12 0:29 ` [PATCH 1/4] Documentation: dt: mailbox: Add Aspeed ast2400/2500 bindings Cyril Bur
2017-01-12 0:29 ` Cyril Bur
2017-01-18 20:38 ` Rob Herring
2017-01-18 20:38 ` Rob Herring
2017-01-19 0:05 ` Cyril Bur
2017-01-19 0:05 ` Cyril Bur
2017-01-19 15:08 ` Benjamin Herrenschmidt
2017-01-19 15:08 ` Benjamin Herrenschmidt
2017-01-12 0:29 ` [PATCH 2/4] Documentation: dt: misc: Add Aspeed ast2400/2500 LPC Control bindings Cyril Bur
2017-01-12 0:29 ` Cyril Bur
2017-01-18 21:16 ` Rob Herring
2017-01-18 21:16 ` Rob Herring
2017-01-19 0:19 ` Cyril Bur
2017-01-19 0:19 ` Cyril Bur
2017-01-12 0:29 ` [PATCH 3/4] drivers/misc: Add ASpeed LPC control driver Cyril Bur
2017-01-12 0:29 ` Cyril Bur
2017-01-12 7:43 ` Greg KH
2017-01-12 7:43 ` Greg KH
2017-01-12 15:36 ` Benjamin Herrenschmidt
2017-01-12 15:36 ` Benjamin Herrenschmidt
2017-01-12 7:47 ` Greg KH
2017-01-12 7:47 ` Greg KH
2017-01-12 10:16 ` Cyril Bur
2017-01-12 10:16 ` Cyril Bur
2017-01-12 10:30 ` Greg KH
2017-01-12 10:30 ` Greg KH
2017-01-12 15:27 ` Benjamin Herrenschmidt [this message]
2017-01-12 15:27 ` Benjamin Herrenschmidt
2017-01-12 16:00 ` Greg KH
2017-01-12 16:00 ` Greg KH
2017-01-12 16:07 ` Benjamin Herrenschmidt
2017-01-12 16:07 ` Benjamin Herrenschmidt
2017-01-12 16:26 ` Greg KH
2017-01-12 16:26 ` Greg KH
2017-01-12 16:31 ` Benjamin Herrenschmidt
2017-01-12 16:31 ` Benjamin Herrenschmidt
2017-01-12 15:35 ` Benjamin Herrenschmidt
2017-01-12 15:35 ` Benjamin Herrenschmidt
2017-01-12 16:27 ` Greg KH
2017-01-12 16:27 ` Greg KH
2017-01-12 16:29 ` Benjamin Herrenschmidt
2017-01-12 16:29 ` Benjamin Herrenschmidt
2017-01-12 17:27 ` Greg KH
2017-01-12 17:27 ` Greg KH
2017-01-12 0:29 ` [PATCH 4/4] drivers/mailbox: Add ASpeed mailbox driver Cyril Bur
2017-01-12 0:29 ` Cyril Bur
2017-02-07 5:40 ` Joel Stanley
2017-02-07 5:40 ` Joel Stanley
2017-02-07 5:44 ` Benjamin Herrenschmidt
2017-02-07 5:44 ` Benjamin Herrenschmidt
2017-02-07 22:57 ` Cyril Bur
2017-02-07 22:57 ` Cyril Bur
2017-02-07 22:59 ` Joel Stanley
2017-02-07 22:59 ` Joel Stanley
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=1484234867.2492.39.camel@kernel.crashing.org \
--to=benh@kernel.crashing.org \
--cc=andrew@aj.id.au \
--cc=arnd@arndb.de \
--cc=cyrilbur@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jassisinghbrar@gmail.com \
--cc=jk@ozlabs.org \
--cc=joel@jms.id.au \
--cc=mark.rutland@arm.com \
--cc=openbmc@lists.ozlabs.org \
--cc=robh+dt@kernel.org \
--cc=xow@google.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.