Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[00/11] 2.6.27.45 review

0 views
Skip to first unread message

Greg KH

unread,
Jan 26, 2010, 2:30:03 PM1/26/10
to
This is the start of the stable review cycle for the 2.6.27.45 release.
There are 11 patches in this series, all will be posted as a response to
this one. If anyone has any issues with these being applied, please let
us know. If anyone is a maintainer of the proper subsystem, and wants
to add a Signed-off-by: line to the patch, please respond with it.

Responses should be made by Thursday, January 28, 20:00:00 UTC.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
kernel.org/pub/linux/kernel/v2.6/stable-review/patch-2.6.27.45-rc1.gz
and the diffstat can be found below.

thanks,

greg k-h

Makefile | 2 +-
arch/s390/kvm/intercept.c | 4 ++--
drivers/char/tty_io.c | 2 +-
drivers/edac/i5000_edac.c | 8 +++++++-
drivers/scsi/megaraid/megaraid_sas.c | 2 +-
drivers/usb/core/hub.c | 3 +++
drivers/usb/host/ehci-hcd.c | 5 +++--
drivers/usb/host/ehci-hub.c | 20 +++++++++++++++++++-
drivers/usb/host/ehci-q.c | 11 ++++++++---
drivers/usb/host/uhci-hcd.c | 15 ++++++++++++++-
drivers/usb/host/uhci-hub.c | 2 +-
fs/ecryptfs/crypto.c | 4 ++--
fs/ecryptfs/file.c | 14 +++++++-------
fs/reiserfs/inode.c | 17 ++++++++++++++---
ipc/msg.c | 1 +
ipc/sem.c | 1 +
ipc/shm.c | 1 +
17 files changed, 86 insertions(+), 26 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Greg KH

unread,
Jan 26, 2010, 2:30:02 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Erez Zadok <e...@cs.sunysb.edu>

commit e27759d7a333d1f25d628c4f7caf845c51be51c2 upstream.

Ecryptfs_open dereferences a pointer to the private lower file (the one
stored in the ecryptfs inode), without checking if the pointer is NULL.
Right afterward, it initializes that pointer if it is NULL. Swap order of
statements to first initialize. Bug discovered by Duckjin Kang.

Signed-off-by: Duckjin Kang <from...@gmail.com>
Signed-off-by: Erez Zadok <e...@cs.sunysb.edu>
Cc: Dustin Kirkland <kirk...@canonical.com>
Cc: Al Viro <vi...@zeniv.linux.org.uk>
Signed-off-by: Tyler Hicks <tyh...@linux.vnet.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
fs/ecryptfs/file.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -192,13 +192,6 @@ static int ecryptfs_open(struct inode *i
| ECRYPTFS_ENCRYPTED);
}
mutex_unlock(&crypt_stat->cs_mutex);
- if ((ecryptfs_inode_to_private(inode)->lower_file->f_flags & O_RDONLY)
- && !(file->f_flags & O_RDONLY)) {
- rc = -EPERM;
- printk(KERN_WARNING "%s: Lower persistent file is RO; eCryptfs "
- "file must hence be opened RO\n", __func__);
- goto out;
- }
if (!ecryptfs_inode_to_private(inode)->lower_file) {
rc = ecryptfs_init_persistent_file(ecryptfs_dentry);
if (rc) {
@@ -209,6 +202,13 @@ static int ecryptfs_open(struct inode *i
goto out;
}
}
+ if ((ecryptfs_inode_to_private(inode)->lower_file->f_flags & O_RDONLY)
+ && !(file->f_flags & O_RDONLY)) {
+ rc = -EPERM;
+ printk(KERN_WARNING "%s: Lower persistent file is RO; eCryptfs "
+ "file must hence be opened RO\n", __func__);
+ goto out;
+ }
ecryptfs_set_file_lower(
file, ecryptfs_inode_to_private(inode)->lower_file);
if (S_ISDIR(ecryptfs_dentry->d_inode->i_mode)) {

Greg KH

unread,
Jan 26, 2010, 2:30:03 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Christian Borntraeger <bornt...@de.ibm.com>

commit 062d5e9b0d714f449b261bb522eadaaf6f00f438 upstream.

kvm_handle_sie_intercept uses a jump table to get the intercept handler
for a SIE intercept. Static code analysis revealed a potential problem:
the intercept_funcs jump table was defined to contain (0x48 >> 2) entries,
but we only checked for code > 0x48 which would cause an off-by-one
array overflow if code == 0x48.

Use the compiler and ARRAY_SIZE to automatically set the limits.

Signed-off-by: Christian Borntraeger <bornt...@de.ibm.com>
Signed-off-by: Marcelo Tosatti <mtos...@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
arch/s390/kvm/intercept.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -199,7 +199,7 @@ static int handle_instruction_and_prog(s
return rc2;
}

-static const intercept_handler_t intercept_funcs[0x48 >> 2] = {
+static const intercept_handler_t intercept_funcs[] = {
[0x00 >> 2] = handle_noop,
[0x04 >> 2] = handle_instruction,
[0x08 >> 2] = handle_prog,
@@ -216,7 +216,7 @@ int kvm_handle_sie_intercept(struct kvm_
intercept_handler_t func;
u8 code = vcpu->arch.sie_block->icptcode;

- if (code & 3 || code > 0x48)
+ if (code & 3 || (code >> 2) >= ARRAY_SIZE(intercept_funcs))
return -ENOTSUPP;
func = intercept_funcs[code >> 2];
if (func)

Greg KH

unread,
Jan 26, 2010, 2:40:02 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Serge E. Hallyn <se...@us.ibm.com>

commit 7d6feeb287c61aafa88f06345387b1188edf4b86 upstream.

We have apparently had a memory leak since
7ca7e564e049d8b350ec9d958ff25eaa24226352 "ipc: store ipcs into IDRs" in
2007. The idr of which 3 exist for each ipc namespace is never freed.

This patch simply frees them when the ipcns is freed. I don't believe any
idr_remove() are done from rcu (and could therefore be delayed until after
this idr_destroy()), so the patch should be safe. Some quick testing
showed no harm, and the memory leak fixed.

Caught by kmemleak.

Signed-off-by: Serge E. Hallyn <se...@us.ibm.com>
Signed-off-by: Andrew Morton <ak...@linux-foundation.org>
Signed-off-by: Linus Torvalds <torv...@linux-foundation.org>
Acked-by: Nick Piggin <npi...@suse.de>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---


ipc/msg.c | 1 +
ipc/sem.c | 1 +
ipc/shm.c | 1 +

3 files changed, 3 insertions(+)

--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -125,6 +125,7 @@ void msg_init_ns(struct ipc_namespace *n
void msg_exit_ns(struct ipc_namespace *ns)
{
free_ipcs(ns, &msg_ids(ns), freeque);
+ idr_destroy(&ns->ids[IPC_MSG_IDS].ipcs_idr);
}
#endif

--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -129,6 +129,7 @@ void sem_init_ns(struct ipc_namespace *n
void sem_exit_ns(struct ipc_namespace *ns)
{
free_ipcs(ns, &sem_ids(ns), freeary);
+ idr_destroy(&ns->ids[IPC_SEM_IDS].ipcs_idr);
}
#endif

--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -100,6 +100,7 @@ static void do_shm_rmid(struct ipc_names
void shm_exit_ns(struct ipc_namespace *ns)
{
free_ipcs(ns, &shm_ids(ns), do_shm_rmid);
+ idr_destroy(&ns->ids[IPC_SHM_IDS].ipcs_idr);
}
#endif

Greg KH

unread,
Jan 26, 2010, 2:40:03 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Alan Stern <st...@rowland.harvard.edu>

commit 1b9a38bfa6e664ff02511314f5586d711c83cc91 upstream.

This patch (as1320) fixes two problems related to interrupt-URB
scheduling in ehci-hcd.

URBs with an interval of 2 or 4 microframes aren't handled.
For the time being, the patch reduces to interval to 1 uframe.

URBs are constrained to have an interval no larger than 1024
frames by usb_submit_urb(). But some EHCI controllers allow
use of a schedule as short as 256 frames; for these
controllers we may have to decrease the interval to the
actual schedule length.

The second problem isn't very significant since few devices expose
interrupt endpoints with an interval larger than 256 frames. But the
first problem is critical; it will prevent the kernel from working
with devices having interrupt intervals of 2 or 4 uframes.

Signed-off-by: Alan Stern <st...@rowland.harvard.edu>
Tested-by: Glynn Farrow <far...@sg.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
drivers/usb/host/ehci-q.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -748,9 +748,10 @@ qh_make (
* But interval 1 scheduling is simpler, and
* includes high bandwidth.
*/
- dbg ("intr period %d uframes, NYET!",
- urb->interval);
- goto done;
+ urb->interval = 1;
+ } else if (qh->period > ehci->periodic_size) {
+ qh->period = ehci->periodic_size;
+ urb->interval = qh->period << 3;
}
} else {
int think_time;
@@ -773,6 +774,10 @@ qh_make (
usb_calc_bus_time (urb->dev->speed,
is_input, 0, max_packet (maxp)));
qh->period = urb->interval;
+ if (qh->period > ehci->periodic_size) {
+ qh->period = ehci->periodic_size;
+ urb->interval = qh->period;
+ }

Greg KH

unread,
Jan 26, 2010, 2:40:03 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Alan Stern <st...@rowland.harvard.edu>

commit 49d0f078f494b9d81e820a13dd8093a9bfb0b6b1 upstream.

This patch (as1330) fixes a bug in khbud's handling of remote
wakeups. When a device sends a remote-wakeup request, the parent hub
(or the host controller driver, for directly attached devices) begins
the resume sequence and notifies khubd when the sequence finishes. At
this point the port's SUSPEND feature is automatically turned off.

However the device needs an additional 10-ms resume-recovery time
(TRSMRCY in the USB spec). Khubd does not wait for this delay if the
SUSPEND feature is off, and as a result some devices fail to behave
properly following a remote wakeup. This patch adds the missing
delay to the remote-wakeup path.

It also extends the resume-signalling delay used by ehci-hcd and
uhci-hcd from 20 ms (the value in the spec) to 25 ms (the value we use
for non-remote-wakeup resumes). The extra time appears to help some
devices.

Signed-off-by: Alan Stern <st...@rowland.harvard.edu>
Cc: Rickard Bellini <rickard...@ericsson.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---


drivers/usb/core/hub.c | 3 +++
drivers/usb/host/ehci-hcd.c | 5 +++--

drivers/usb/host/uhci-hub.c | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)

--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3015,6 +3015,9 @@ static void hub_events(void)
USB_PORT_FEAT_C_SUSPEND);
udev = hdev->children[i-1];
if (udev) {
+ /* TRSMRCY = 10 msec */
+ msleep(10);
+
usb_lock_device(udev);
ret = remote_wakeup(hdev->
children[i-1]);
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -723,9 +723,10 @@ static irqreturn_t ehci_irq (struct usb_

/* start 20 msec resume signaling from this port,
* and make khubd collect PORT_STAT_C_SUSPEND to
- * stop that signaling.
+ * stop that signaling. Use 5 ms extra for safety,
+ * like usb_port_resume() does.
*/
- ehci->reset_done [i] = jiffies + msecs_to_jiffies (20);
+ ehci->reset_done[i] = jiffies + msecs_to_jiffies(25);
ehci_dbg (ehci, "port %d remote wakeup\n", i + 1);
mod_timer(&hcd->rh_timer, ehci->reset_done[i]);
}
--- a/drivers/usb/host/uhci-hub.c
+++ b/drivers/usb/host/uhci-hub.c
@@ -167,7 +167,7 @@ static void uhci_check_ports(struct uhci
/* Port received a wakeup request */
set_bit(port, &uhci->resuming_ports);
uhci->ports_timeout = jiffies +
- msecs_to_jiffies(20);
+ msecs_to_jiffies(25);

/* Make sure we see the port again
* after the resuming period is over. */

Greg KH

unread,
Jan 26, 2010, 2:40:02 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Alan Stern <st...@rowland.harvard.edu>

commit cec3a53c7fe794237b582e8e77fc0e48465e65ee upstream.

This patch (as1321) fixes a problem with EHCI and UHCI root-hub
suspends: If the suspend occurs while a port is trying to resume, the
resume doesn't finish and simply gets lost. When remote wakeup is
enabled, this is undesirable behavior.

The patch checks first to see if any port resumes are in progress, and
if they are then it fails the root-hub suspend with -EBUSY.

Signed-off-by: Alan Stern <st...@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
drivers/usb/host/ehci-hub.c | 20 +++++++++++++++++++-
drivers/usb/host/uhci-hcd.c | 15 ++++++++++++++-
2 files changed, 33 insertions(+), 2 deletions(-)

--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -119,9 +119,26 @@ static int ehci_bus_suspend (struct usb_
del_timer_sync(&ehci->watchdog);
del_timer_sync(&ehci->iaa_watchdog);

- port = HCS_N_PORTS (ehci->hcs_params);
spin_lock_irq (&ehci->lock);

+ /* Once the controller is stopped, port resumes that are already
+ * in progress won't complete. Hence if remote wakeup is enabled
+ * for the root hub and any ports are in the middle of a resume or
+ * remote wakeup, we must fail the suspend.
+ */
+ if (hcd->self.root_hub->do_remote_wakeup) {
+ port = HCS_N_PORTS(ehci->hcs_params);
+ while (port--) {
+ if (ehci->reset_done[port] != 0) {
+ spin_unlock_irq(&ehci->lock);
+ ehci_dbg(ehci, "suspend failed because "
+ "port %d is resuming\n",
+ port + 1);
+ return -EBUSY;
+ }
+ }
+ }
+
/* stop schedules, clean any completed work */
if (HC_IS_RUNNING(hcd->state)) {
ehci_quiesce (ehci);
@@ -137,6 +154,7 @@ static int ehci_bus_suspend (struct usb_
*/
ehci->bus_suspended = 0;
ehci->owned_ports = 0;
+ port = HCS_N_PORTS(ehci->hcs_params);
while (port--) {
u32 __iomem *reg = &ehci->regs->port_status [port];
u32 t1 = ehci_readl(ehci, reg) & ~PORT_RWC_BITS;
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -750,7 +750,20 @@ static int uhci_rh_suspend(struct usb_hc
spin_lock_irq(&uhci->lock);
if (!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags))
rc = -ESHUTDOWN;
- else if (!uhci->dead)
+ else if (uhci->dead)
+ ; /* Dead controllers tell no tales */
+
+ /* Once the controller is stopped, port resumes that are already
+ * in progress won't complete. Hence if remote wakeup is enabled
+ * for the root hub and any ports are in the middle of a resume or
+ * remote wakeup, we must fail the suspend.
+ */
+ else if (hcd->self.root_hub->do_remote_wakeup &&
+ uhci->resuming_ports) {
+ dev_dbg(uhci_dev(uhci), "suspend failed because a port "
+ "is resuming\n");
+ rc = -EBUSY;
+ } else
suspend_rh(uhci, UHCI_RH_SUSPENDED);
spin_unlock_irq(&uhci->lock);
return rc;

Greg KH

unread,
Jan 26, 2010, 2:40:03 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Dan Carpenter <err...@gmail.com>

commit ece550f51ba175c14ec3ec047815927d7386ea1f upstream.

The "full_alg_name" variable is used on a couple error paths, so we
shouldn't free it until the end.

Signed-off-by: Dan Carpenter <err...@gmail.com>
Signed-off-by: Tyler Hicks <tyh...@linux.vnet.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
fs/ecryptfs/crypto.c | 4 ++--


1 file changed, 2 insertions(+), 2 deletions(-)

--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1733,7 +1733,7 @@ ecryptfs_process_key_cipher(struct crypt
char *cipher_name, size_t *key_size)
{
char dummy_key[ECRYPTFS_MAX_KEY_BYTES];
- char *full_alg_name;
+ char *full_alg_name = NULL;
int rc;

*key_tfm = NULL;
@@ -1748,7 +1748,6 @@ ecryptfs_process_key_cipher(struct crypt
if (rc)
goto out;
*key_tfm = crypto_alloc_blkcipher(full_alg_name, 0, CRYPTO_ALG_ASYNC);
- kfree(full_alg_name);
if (IS_ERR(*key_tfm)) {
rc = PTR_ERR(*key_tfm);
printk(KERN_ERR "Unable to allocate crypto cipher with name "
@@ -1770,6 +1769,7 @@ ecryptfs_process_key_cipher(struct crypt
goto out;
}
out:
+ kfree(full_alg_name);
return rc;

Greg KH

unread,
Jan 26, 2010, 2:40:02 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Bryn M. Reeves <b...@redhat.com>

commit bb7d3f24c71e528989501617651b669fbed798cb upstream.

/sys/bus/pci/drivers/megaraid_sas/poll_mode_io defaults to being
world-writable, which seems bad (letting any user affect kernel driver
behavior).

This turns off group and user write permissions, so that on typical
production systems only root can write to it.

Signed-off-by: Bryn M. Reeves <b...@redhat.com>
Signed-off-by: Linus Torvalds <torv...@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
drivers/scsi/megaraid/megaraid_sas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -3462,7 +3462,7 @@ out:
return retval;
}

-static DRIVER_ATTR(poll_mode_io, S_IRUGO|S_IWUGO,
+static DRIVER_ATTR(poll_mode_io, S_IRUGO|S_IWUSR,
megasas_sysfs_show_poll_mode_io,
megasas_sysfs_set_poll_mode_io);

Greg KH

unread,
Jan 26, 2010, 2:40:02 PM1/26/10
to
2.6.27-stable review patch. If anyone has any objections, please let us know.

------------------

From: Greg Kroah-Hartman <gre...@suse.de>

commit 703625118069f9f8960d356676662d3db5a9d116 upstream.

We need to keep the lock held over the call to __f_setown() to
prevent a PID race.

Thanks to Al Viro for pointing out the problem, and to Travis for
making us look here in the first place.

Cc: Eric W. Biederman <ebie...@xmission.com>
Cc: Al Viro <vi...@ZenIV.linux.org.uk>
Cc: Alan Cox <al...@lxorguk.ukuu.org.uk>
Cc: Linus Torvalds <torv...@linux-foundation.org>
Cc: Tavis Ormandy <tav...@google.com>
Cc: Jeff Dike <jd...@addtoit.com>
Cc: Julien Tinnes <j...@google.com>
Cc: Matt Mackall <m...@selenic.com>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

---
drivers/char/tty_io.c | 2 +-


1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2437,8 +2437,8 @@ static int tty_fasync(int fd, struct fil
pid = task_pid(current);
type = PIDTYPE_PID;
}
- spin_unlock_irqrestore(&tty->ctrl_lock, flags);
retval = __f_setown(filp, pid, type, 0);
+ spin_unlock_irqrestore(&tty->ctrl_lock, flags);
if (retval)
goto out;
} else {

Eric W. Biederman

unread,
Jan 26, 2010, 3:00:01 PM1/26/10
to
Greg KH <gre...@suse.de> writes:

> 2.6.27-stable review patch. If anyone has any objections, please let us know.

Only that __f_setown by way of f_modown unconditionally enables interrupts. So
without touching f_modown as well in mainline we have nasty sounding lockdep warnings.

Eric

Linus Torvalds

unread,
Jan 26, 2010, 5:20:02 PM1/26/10
to

On Tue, 26 Jan 2010, Eric W. Biederman wrote:

> Greg KH <gre...@suse.de> writes:
>
> > 2.6.27-stable review patch. If anyone has any objections, please let us know.
>
> Only that __f_setown by way of f_modown unconditionally enables interrupts. So
> without touching f_modown as well in mainline we have nasty sounding lockdep warnings.

Hmm. That seems to be true in mainline too, isn't it?

So now we have:
- tty_fasync() gets tty->ctrl_lock, with spin_lock_irqsave()

- it then calls __f_setown()

- which calls f_modown(),

- which does

write_lock_irq(&filp->f_owner.lock);
..
write_unlock_irq(&filp->f_owner.lock);

which in turn enables interrupts while we still hold ctrl_lock.

so that whole commit 70362511806 was/is buggy in mainline too.

The minimal fix is likely to just make f_modown() use
write_lock_irqsave/restore. Greg?

Linus

Eric W. Biederman

unread,
Jan 26, 2010, 6:10:02 PM1/26/10
to
Linus Torvalds <torv...@linux-foundation.org> writes:

> On Tue, 26 Jan 2010, Eric W. Biederman wrote:
>
>> Greg KH <gre...@suse.de> writes:
>>
>> > 2.6.27-stable review patch. If anyone has any objections, please let us know.
>>
>> Only that __f_setown by way of f_modown unconditionally enables interrupts. So
>> without touching f_modown as well in mainline we have nasty sounding lockdep warnings.
>
> Hmm. That seems to be true in mainline too, isn't it?

Yes. We are having that conversation in the thread:
"[2.6.33-rc5] starting emacs makes lockdep warning"

Eric

Greg KH

unread,
Jan 26, 2010, 6:10:02 PM1/26/10
to
On Tue, Jan 26, 2010 at 02:11:28PM -0800, Linus Torvalds wrote:
>
>
> On Tue, 26 Jan 2010, Eric W. Biederman wrote:
>
> > Greg KH <gre...@suse.de> writes:
> >
> > > 2.6.27-stable review patch. If anyone has any objections, please let us know.
> >
> > Only that __f_setown by way of f_modown unconditionally enables interrupts. So
> > without touching f_modown as well in mainline we have nasty sounding lockdep warnings.
>
> Hmm. That seems to be true in mainline too, isn't it?
>
> So now we have:
> - tty_fasync() gets tty->ctrl_lock, with spin_lock_irqsave()
>
> - it then calls __f_setown()
>
> - which calls f_modown(),
>
> - which does
>
> write_lock_irq(&filp->f_owner.lock);
> ..
> write_unlock_irq(&filp->f_owner.lock);
>
> which in turn enables interrupts while we still hold ctrl_lock.
>
> so that whole commit 70362511806 was/is buggy in mainline too.
>
> The minimal fix is likely to just make f_modown() use
> write_lock_irqsave/restore. Greg?

Yes, that looks correct.

Here's a patch that does just that:
---------

From: Greg Kroah-Hartman <gre...@suse.de>
Subject: fnctl: f_modown should call write_lock_irqsave/restore

Commit 703625118069f9f8960d356676662d3db5a9d116 exposed that f_modown()
should call write_lock_irqsave instead of just write_lock_irq so that
because a caller could have a spinlock held and it would not be good to
renable interrupts.

Cc: Eric W. Biederman <ebie...@xmission.com>
Cc: Al Viro <vi...@ZenIV.linux.org.uk>
Cc: Alan Cox <al...@lxorguk.ukuu.org.uk>

Cc: Tavis Ormandy <tav...@google.com>
Cc: stable <sta...@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gre...@suse.de>

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 97e01dc..5ef953e 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -199,7 +199,9 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
int force)
{
- write_lock_irq(&filp->f_owner.lock);
+ unsigned long flags;
+
+ write_lock_irqsave(&filp->f_owner.lock, flags);
if (force || !filp->f_owner.pid) {
put_pid(filp->f_owner.pid);
filp->f_owner.pid = get_pid(pid);
@@ -211,7 +213,7 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
filp->f_owner.euid = cred->euid;
}
}
- write_unlock_irq(&filp->f_owner.lock);
+ write_unlock_irqrestore(&filp->f_owner.lock, flags);
}

int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,

Linus Torvalds

unread,
Jan 26, 2010, 8:40:01 PM1/26/10
to

On Tue, 26 Jan 2010, Greg KH wrote:
> From: Greg Kroah-Hartman <gre...@suse.de>
> Subject: fnctl: f_modown should call write_lock_irqsave/restore

Ok, applied and pushed out as b04da8b, so you can add it to the -stable
list.

Linus

Greg KH

unread,
Jan 26, 2010, 8:50:01 PM1/26/10
to
On Tue, Jan 26, 2010 at 05:30:46PM -0800, Linus Torvalds wrote:
>
>
> On Tue, 26 Jan 2010, Greg KH wrote:
> > From: Greg Kroah-Hartman <gre...@suse.de>
> > Subject: fnctl: f_modown should call write_lock_irqsave/restore
>
> Ok, applied and pushed out as b04da8b, so you can add it to the -stable
> list.

Thanks, I'll go queue it up now.

greg k-h

0 new messages