linux-kernel - Re: [git pull] vfs.git: poll annotations

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: <20180201220001.GE30522@ZenIV.linux.org.uk>
Date:   Thu, 1 Feb 2018 22:00:01 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [git pull] vfs.git: poll annotations

On Wed, Jan 31, 2018 at 04:46:20AM +0000, Al Viro wrote:
> On Tue, Jan 30, 2018 at 07:19:19PM -0800, Linus Torvalds wrote:
> > On Tue, Jan 30, 2018 at 7:13 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> > >
> > > As for the place for mangle/demangle... where would you prefer them?
> > 
> > If we can make gcc generate ok code for the generic case and not need
> > any arch-specific ones, why not just do in <linux/poll.h>?
> 
> Umm...  I'll try and see how well does that work.

FWIW, it does generate sane code and bulk rename itself is not hard.
What I've got is in vfs.git#work.poll:
      smc: missing poll annotations
      xen: fix poll misannotation
      use linux/poll.h instead of asm/poll.h
      add EPOLLNVAL, annotate EPOLL... and event_poll->event
      preparation to switching ->poll() to returning EPOLL...
      ep_send_events_proc(): return result via esed->res
      annotate ep_scan_ready_list()
      switch users of kernel-side POLL... to EPOLL...
      unify {de,}mangle_poll(), get rid of kernel-side POLL...
The first 7 commits are preparation, then mechanical bulk rename with
for V in IN OUT PRI ERR RDNORM RDBAND WRNORM WRBAND HUP RDHUP NVAL MSG; do
  L=`git grep -l -w POLL$V | grep -v '^t' | grep -v /um/ | grep -v '^sa' | grep -v '/poll.h$'|grep -v '^D'`
  for f in $L; do sed -i "-es/^\([^\"]*\)\(\<POLL$V\>\)/\\1E\\2/" $f; done
done
then consolidation of mangle_poll/demangle_poll and making those
ifndef __KERNEL__ poll.h unconditional.

FWIW, with the entire series applied __poll_t warnings (with CF=-D__CHECK_POLL)
are down to the following:

* genuine bugs in ->poll() instances, of the "return -EINVAL/-ERESTARTSYS/etc."
sort -
kernel/trace/ring_buffer.c:643:32:    expected restricted __poll_t
drivers/media/pci/saa7164/saa7164-vbi.c:632:24:    expected restricted __poll_t
drivers/media/pci/saa7164/saa7164-vbi.c:637:40:    expected restricted __poll_t
drivers/media/pci/saa7164/saa7164-vbi.c:647:40:    expected restricted __poll_t
drivers/media/platform/exynos-gsc/gsc-m2m.c:718:24:    expected restricted __poll_t
drivers/media/platform/s3c-camif/camif-capture.c:602:21:    expected restricted __poll_t [usertype] ret
drivers/media/radio/radio-wl1273.c:1099:24:    expected restricted __poll_t
drivers/platform/goldfish/goldfish_pipe.c:549:24:    expected restricted __poll_t
drivers/tty/n_r3964.c:1239:24:    expected restricted __poll_t [assigned] [usertype] result
drivers/uio/uio.c:505:24:    expected restricted __poll_t
drivers/xen/pvcalls-front.c:921:24:    expected restricted __poll_t
sound/core/seq/seq_clientmgr.c:1097:24:    expected restricted __poll_t
sound/core/seq/oss/seq_oss.c:206:24:    expected restricted __poll_t

* misannotation around SAA6588_CMD_POLL and struct saa6588_command -
drivers/media/i2c/saa6588.c:416:35:    right side has type restricted __poll_t
drivers/media/pci/bt8xx/bttv-driver.c:3347:20:    got restricted __poll_t [assigned] [usertype] res
drivers/media/pci/bt8xx/bttv-driver.c:3350:19:    expected restricted __poll_t
drivers/media/pci/saa7134/saa7134-video.c:1243:16: warning: restricted __poll_t degrades to integer
drivers/media/pci/saa7134/saa7134-video.c:1243:19:    expected restricted __poll_t

* misannotations around perf ring_buffer->poll -
kernel/events/core.c:4541:24:    expected restricted __poll_t [usertype] events
kernel/events/ring_buffer.c:22:39:    got restricted __poll_t [usertype] <noident>

IOW, not much and fairly low on false positives.

Are you OK with that approach?  I could send a pull request for the first 7
in that series (it should be bisect-safe), then you'd run the bulk rename
script just before -rc1 and apply the last one on top of that.