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] [thread-next>] [day] [month] [year] [list]
Message-ID: <16082.1210952646@vena.lwn.net>
Date:	Fri, 16 May 2008 09:44:06 -0600
From:	corbet@....net (Jonathan Corbet)
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Alexander Viro <viro@....linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: [PATCH, RFC] char dev BKL pushdown 

Linus said:

> So it literally should be:
>  - remove one lock_kernel/unlock_kernel pair in fs/char_dev.c
>  - add max 83 pairs in the places that register those things

...and so that's what I've done.  My approach was to find every
register_chrdev() and cdev_add() call, look at the associated
file_operations, then go back to the open() function, if any.  Unless it
was almost immediately obvious to me that the function was either (1) so
trivial as to not require locking (quite few of them are "return 0;"), or
(2) clearly doing its own locking, I wrapped the code in the BKL.

Finally, I removed the BKL from chrdev_open().

Allmodconfig and allyesconfig makes work here, and this kernel runs on my
dual-core desktop.  But, clearly, I don't have all of this hardware.
Actually, I wonder if some of it still exists outside of museums.  So
there's probably something stupid in there somewhere.

The result is available at:

	git://git.lwn.net/linux-2.6.git cdev

I'll put a shortlog and diffstat at the end of this message.  For
completeness, there's also a list of files I examined and did *not* change.

Assuming nobody tells me I'm completely off-base, I guess my next step is
to start running individual patches past maintainers.  Some of them,
probably (I hope), will tell me that I've been wasting my time and that
their code doesn't need the BKL.  In such cases, I'll gladly drop the
associated patch.  But there's a fair amount of stuff here which clearly
*does* need it still.  If all seems well, maybe this tree should get into
linux-next at some point too.

Comments?

The changes come out to this:

 arch/cris/arch-v10/drivers/gpio.c         |    3 ++
 arch/cris/arch-v10/drivers/sync_serial.c  |   34 ++++++++++++++++----------
 arch/cris/arch-v32/drivers/mach-a3/gpio.c |    4 +++
 arch/cris/arch-v32/drivers/mach-fs/gpio.c |    5 +++
 arch/cris/arch-v32/drivers/sync_serial.c  |   33 +++++++++++++++-----------
 arch/mips/kernel/rtlx.c                   |    7 ++++-
 arch/mips/kernel/vpe.c                    |   12 +++++++--
 arch/mips/sibyte/common/sb_tbprof.c       |   25 ++++++++++++++-----
 arch/sh/boards/landisk/gio.c              |   10 +++++--
 arch/x86/kernel/cpuid.c                   |   25 +++++++++++++------
 arch/x86/kernel/msr.c                     |   16 +++++++++---
 block/bsg.c                               |    7 ++++-
 drivers/block/aoe/aoechr.c                |    7 ++++-
 drivers/block/paride/pg.c                 |   22 ++++++++++++-----
 drivers/block/paride/pt.c                 |    8 +++++-
 drivers/char/drm/drm_fops.c               |    9 +++++--
 drivers/char/ipmi/ipmi_devintf.c          |    8 ++++--
 drivers/char/lp.c                         |   38 ++++++++++++++++++++----------
 drivers/char/mbcs.c                       |    5 +++
 drivers/char/mem.c                        |   10 ++++++-
 drivers/char/misc.c                       |    3 ++
 drivers/char/pcmcia/cm4000_cs.c           |   26 +++++++++++++++-----
 drivers/char/pcmcia/cm4040_cs.c           |   23 +++++++++++++-----
 drivers/char/snsc.c                       |    5 +++
 drivers/char/tty_io.c                     |   27 +++++++++++++++++++--
 drivers/char/viotape.c                    |    3 ++
 drivers/firewire/fw-cdev.c                |   16 +++++++++---
 drivers/hid/hidraw.c                      |    3 ++
 drivers/i2c/i2c-dev.c                     |   22 ++++++++++++-----
 drivers/ide/ide-tape.c                    |    7 ++++-
 drivers/ieee1394/dv1394.c                 |    6 +++-
 drivers/ieee1394/raw1394.c                |    3 ++
 drivers/ieee1394/video1394.c              |   18 ++++++++++----
 drivers/input/input.c                     |   16 +++++++++---
 drivers/isdn/i4l/isdn_common.c            |    3 +-
 drivers/media/dvb/dvb-core/dvbdev.c       |    4 +++
 drivers/mtd/mtdchar.c                     |   22 ++++++++++++-----
 drivers/mtd/ubi/cdev.c                    |    7 ++++-
 drivers/net/wan/cosa.c                    |   22 ++++++++++++-----
 drivers/pcmcia/pcmcia_ioctl.c             |   25 ++++++++++++++-----
 drivers/rtc/rtc-dev.c                     |   12 +++++++--
 drivers/s390/char/fs3270.c                |   23 ++++++++++++------
 drivers/s390/char/tape_char.c             |   12 +++++++--
 drivers/s390/char/vmlogrdr.c              |    8 +++++-
 drivers/s390/char/vmur.c                  |   12 +++++++--
 drivers/scsi/aacraid/linit.c              |    3 ++
 drivers/scsi/gdth.c                       |    3 ++
 drivers/scsi/osst.c                       |   15 +++++++++++
 drivers/scsi/sg.c                         |   16 ++++++++++--
 drivers/scsi/st.c                         |   11 +++++++-
 drivers/telephony/phonedev.c              |    3 ++
 drivers/uio/uio.c                         |   17 +++++++++----
 drivers/usb/core/file.c                   |    3 ++
 drivers/video/fbmem.c                     |   15 ++++++++---
 fs/char_dev.c                             |    8 ++----
 sound/core/sound.c                        |   15 +++++++++++
 sound/sound_core.c                        |    5 +++
 57 files changed, 554 insertions(+), 176 deletions(-)

The associated shortlog is:

Jonathan Corbet (43):
      bsg: cdev lock_kernel() pushdown
      cris: cdev lock_kernel() pushdown
      mips: cdev lock_kernel() pushdown
      sh: cdev lock_kernel() pushdown
      x86: cdev lock_kernel() pushdown
      i2c: cdev lock_kernel() pushdown
      cosa: cdev lock_kernel() pushdown
      pcmcia: cdev lock_kernel() pushdown
      ieee1394: cdev lock_kernel() pushdown
      rtc: cdev lock_kernel() pushdown
      drivers/s390: cdev lock_kernel() pushdown
      AoE: cdev lock_kernel() pushdown
      paride: cdev lock_kernel() pushdown
      mtdchar: cdev lock_kernel() pushdown
      UBI: cdev lock_kernel() pushdown
      firewire: cdev lock_kernel() pushdown
      HID: cdev lock_kernel() pushdown
      Input: cdev lock_kernel() pushdown
      UIO: cdev lock_kernel() pushdown
      cm40x0: cdev lock_kernel() pushdown
      ipmi: cdev lock_kernel() pushdown
      mem: cdev lock_kernel() pushdown
      misc: cdev lock_kernel() pushdown
      viotape: cdev lock_kernel pushdown
      mbcs: cdev lock_kernel() pushdown
      lp: cdev lock_kernel() pushdown
      drm: cdev lock_kernel() pushdown
      phonedev: cdev lock_kernel() pushdown
      ide-tape: cdev lock_kernel() pushdown
      sg: cdev lock_kernel() pushdown
      osst: cdev lock_kernel() pushdown.
      aacraid: cdev lock_kernel() pushdown
      st: cdev lock_kernel() pushdown
      gdth: cdev lock_kernel() pushdown
      isdn: cdev lock_kernel() pushdown
      usbcore: cdev lock_kernel() pushdown
      dvb: cdev lock_kernel() pushdown
      fbmem: cdev lock_kernel() pushdown
      sound: cdev lock_kernel() pushdown
      snsc: cdev lock_kernel() pushdown
      tty: cdev lock_kernel() pushdown
      Remove the lock_kernel() call from chrdev_open()
      Add a comment in chrdev_open()

Char device source files which I did *not* change:

	arch/cris/arch-v32/drivers/pcf8563.c (no open function)
	arch/cris/arch-v32/drivers/i2c.c (empty open function)
	arch/cris/arch-v32/drivers/cryptocop.c (almost-empty open function)
	arch/cris/arch-v10/drivers/pcf8563.c  (no open function)
	arch/cris/arch-v10/drivers/i2c.c (empty open function)
	arch/cris/arch-v10/drivers/ds1302.c (no open function)
	arch/cris/arch-v10/drivers/eeprom.c (trivial)
	drivers/net/ppp_generic.c (trivial)
	drivers/ieee1394/ieee1394_core.c (almost-trivial)
	drivers/spi/spidev.c  (locking looks right)
	drivers/char/cs5535_gpio.c (trivial)
	drivers/char/dtlk.c (trivial, broken open)
	drivers/char/ds1302.c (no open)
	drivers/char/pc8736x_gpio.c (trivial)
	drivers/char/stallion.c (no open)
	drivers/char/tb0219.c (trivial open)
	drivers/char/vc_screen.c (trivial open)
	drivers/char/ppdev.c (trivial open)
	drivers/char/ip2/ip2main.c (trivial open - does not do anything!)
	drivers/char/scx200_gpio.c (trivial open)
	drivers/char/xilinx_hwicap/xilinx_hwicap.c (locking looks right)
	drivers/char/istallion.c (no open)
	drivers/char/vr41xx_giu.c (trivial)
	drivers/char/tlclk.c (locking looks right)
	drivers/char/raw.c (obvious locking w/raw_mutex)
	drivers/char/dsp56k.c (single-use locking)
	drivers/infiniband/hw/ipath/ipath_file_ops.c (trivial open)
	drivers/infiniband/core/user_mad.c (appears to have good locking)
	drivers/infiniband/core/uverbs_main.c (ditto)
	drivers/infiniband/core/ucm.c (trivial open)
	drivers/misc/phantom.c (has locking)
	drivers/sbus/char/bpp.c (has locking)
	drivers/sbus/char/vfc_dev.c (has locking)
	drivers/scsi/dpt_i2o.c (has locking)
	drivers/scsi/3w-xxxx.c (trivial open)
	drivers/scsi/3w-9xxx.c (trivial open)
	drivers/scsi/megaraid/megaraid_sas.c (trivial open)
	drivers/scsi/ch.c (has locking)
	drivers/scsi/megaraid.c (trivial open)
	drivers/isdn/capi/capi.c (semi-trivial open)
	drivers/isdn/hardware/eicon/divasi.c (empty open)
	drivers/isdn/hardware/eicon/divamnt.c (single-use open)
	drivers/isdn/hardware/eicon/divasmain.c (empty open)
	drivers/macintosh/adb.c (trivial open)
	drivers/usb/gadget/printer.c (has locking)
	drivers/usb/mon/mon_bin.c (has locking)
	drivers/usb/core/endpoint.c (no opens)
	drivers/usb/core/devio.c (has locking)
	drivers/media/video/videodev.c (has locking)
	fs/coda/psdev.c (already has lock_kernel() calls)

jon
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ