public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: linux-kernel@vger.kernel.org
Subject: [RFC] ->poll() bugs
Date: Thu, 5 Mar 2015 15:49:21 +0000	[thread overview]
Message-ID: <20150305154921.GZ29656@ZenIV.linux.org.uk> (raw)

	Several days ago there was an interesting bug in a gadgetfs patch
posted on linux-usb - ->poll() instance returning a negative value on
what it considered an error.  The trouble is, callers of ->poll() expect
a bitmap, not an integer.  Reaction to small negative integer returned
by it is quite bogus.  The bug wasn't hard to spot and fix (the value we
wanted had been the same as if ->poll() had been NULL, i.e. DEFAULT_POLLMASK).
However, it looked as a very easily repeated error.

	I went looking through other ->poll() instances searching for more
of the same and caught sveral more.  Some random examples:

sound/oss/dmasound/dmasound_core.c:
static unsigned int sq_poll(struct file *file, struct poll_table_struct *wait)
{
        unsigned int mask = 0;
        int retVal;

        if (write_sq.locked == 0) {
                if ((retVal = sq_setup(&write_sq)) < 0)
                        return retVal;

sound/core/pcm_native.c:
static unsigned int snd_pcm_capture_poll(struct file *file, poll_table * wait)
{
        struct snd_pcm_file *pcm_file;
        struct snd_pcm_substream *substream;
        struct snd_pcm_runtime *runtime;
        unsigned int mask;
        snd_pcm_uframes_t avail;

        pcm_file = file->private_data;

        substream = pcm_file->substream;
        if (PCM_RUNTIME_CHECK(substream))
                return -ENXIO;

arch/powerpc/platforms/cell/spufs/file.c:
static unsigned int spufs_switch_log_poll(struct file *file, poll_table *wait)
{
        struct inode *inode = file_inode(file);
        struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
        unsigned int mask = 0;
        int rc;

        poll_wait(file, &ctx->switch_log->wait, wait);

        rc = spu_acquire(ctx);
        if (rc)
                return rc;
(spu_acquire() can return -EINTR), etc.

We obviously want to return something saner for all of those.  The interesting
question is what value to return; AFAICS it really depends upon the driver.

I wonder what could be done to catch the crap of that sort; an obvious
solution would be to declare a __bitwise type (poll_t, or something like that),
add force-casts to that type into definitions of constants (conditional upon
__CHECKER__ - these guys are in uapi/linux/poll.h) have ->poll() made to
return that and let sparse catch such places.  Will be quite a bit of
churn, but then it doesn't have to be done all at once - e.g.
#ifdef CHECKER_POLL
typedef unsigned int __bitwise poll_t;
#else
typedef unsigned int poll_t;
#endif
in linux/types.h,
	poll_t (*poll)(.....)
in linux/fs.h
and
#ifdef CHECKER_POLL

             reply	other threads:[~2015-03-05 15:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-05 15:49 Al Viro [this message]
2015-03-05 15:58 ` [RFC] ->poll() bugs Al Viro
2015-03-07 20:44 ` Al Viro
2015-03-07 20:53   ` Linus Torvalds
2015-03-07 21:03     ` Al Viro
2015-03-07 21:08     ` [PATCH] sunrpc: fix braino in ->poll() Al Viro
2015-03-09 19:41       ` J. Bruce Fields
2015-03-08  1:33     ` [RFC] ->poll() bugs Al Viro

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=20150305154921.GZ29656@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-kernel@vger.kernel.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