linux-kernel - [RFC] ->poll() bugs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Mar 2015 15:49:21 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	linux-kernel@...r.kernel.org
Subject: [RFC] ->poll() bugs

	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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/