netdev - Re: [PATCH 03/31] fs: introduce new ->get_poll_head and ->poll

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-prev] [day] [month] [year] [list]

Message-ID: <20180110174240.GA7976@lst.de>
Date:   Wed, 10 Jan 2018 18:42:40 +0100
From:   Christoph Hellwig <hch@....de>
To:     Al Viro <viro@...IV.linux.org.uk>
Cc:     Christoph Hellwig <hch@....de>, Avi Kivity <avi@...lladb.com>,
        linux-aio@...ck.org, linux-fsdevel@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/31] fs: introduce new ->get_poll_head and
        ->poll_mask methods

On Wed, Jan 10, 2018 at 04:31:35PM +0000, Al Viro wrote:
> *snort*
> 
> Seeing that random drivers are, by far, the majority of instances...
> What I wonder is how many of them conform to that pattern and how
> many can be massaged to that form.
> 
> How painful would it be, to pick an instance with more than one wait
> queue involved, to convert drivers/char/random.c to that form?

Attached.  Not very painful at all.  Would be even nicer if we had a
wait_event version that can deal with keys, but I can look into that.

> static unsigned int vtpm_proxy_fops_poll(struct file *filp, poll_table *wait)
> {
>         struct proxy_dev *proxy_dev = filp->private_data;
>         unsigned ret;
> 
>         poll_wait(filp, &proxy_dev->wq, wait);
> 
>         ret = POLLOUT;
> 
>         mutex_lock(&proxy_dev->buf_lock);
> 
>         if (proxy_dev->req_len)
>                 ret |= POLLIN | POLLRDNORM;
> 
>         if (!(proxy_dev->state & STATE_OPENED_FLAG))
>                 ret |= POLLHUP;
> 
>         mutex_unlock(&proxy_dev->buf_lock);
> 
>         return ret;
> } 
> (mainline drivers/char/tpm/tpm_vtpm_proxy.c)

Yeah.  And what exactly is the lock protecting given that each
of them checks a single smaller than register sized variable?

> Is that mutex_lock() in there a bug?  Another fun case is dma_buf_poll()...

Right now it is not a bug, but it is not very helpful behavior.  We
actually have quite a few of those, and not allowing ->poll_mask to
is a good thing to catch these.

View attachment "0001-random-convert-to-poll_mask.patch" of type "text/x-patch" (3747 bytes)