2.6.27.45 review

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(-)

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

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

From: Tamas Vincze <[email protected]>

commit 118f3e1afd5534c15f9701f33514186cfc841a27 upstream.

EDAC MC0: INTERNAL ERROR: channel-b out of range (4 >= 4)

Kernel panic - not syncing: EDAC MC0: Uncorrected Error  (XEN) Domain 0 crashed: 'noreboot' set - not rebooting.

This happens because FERR_NF_FBD bit 28 is not updated on i5000. Due to that, both bits 28 and 29 may be equal to one, returning channel = 3. As this value is invalid, EDAC core generates the panic.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=14568

---
 drivers/edac/i5000_edac.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

@@ -566,7 +566,13 @@ static void i5000_process_nonfatal_error
 		debugf0("\tUncorrected bits= 0x%x\n", ue_errors);
 
 		branch = EXTRACT_FBDCHAN_INDX(info->ferr_nf_fbd);
-		channel = branch;
+
+		/*
+		 * According with i5000 datasheet, bit 28 has no significance
+		 * for errors M4Err-M12Err and M17Err-M21Err, on FERR_NF_FBD
+		 */
+		channel = branch & 2;
+
 		bank = NREC_BANK(info->nrecmema);
 		rank = NREC_RANK(info->nrecmema);
 		rdwr = NREC_RDWR(info->nrecmema);

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

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

From: Bryn M. Reeves <[email protected]>

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.

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

@@ -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);
 

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

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

From: Jan Kara <[email protected]>

commit ec8e2f7466ca370f5e09000ca40a71759afc9ac8 upstream.

It can happen that write does not use all the blocks allocated in write_begin either because of some filesystem error (like ENOSPC) or because page with data to write has been removed from memory. We truncate these blocks so that we don't have dangling blocks beyond i_size.

---
 fs/reiserfs/inode.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

@@ -2545,6 +2545,12 @@ static int reiserfs_writepage(struct pag
 	return reiserfs_write_full_page(page, wbc);
 }
 
+static void reiserfs_truncate_failed_write(struct inode *inode)
+{
+	truncate_inode_pages(inode->i_mapping, inode->i_size);
+	reiserfs_truncate_file(inode, 0);
+}
+
 static int reiserfs_write_begin(struct file *file,
 				struct address_space *mapping,
 				loff_t pos, unsigned len, unsigned flags,

@@ -2611,6 +2617,8 @@ static int reiserfs_write_begin(struct f
 	if (ret) {
 		unlock_page(page);
 		page_cache_release(page);
+		/* Truncate allocated blocks */
+		reiserfs_truncate_failed_write(inode);
 	}
 	return ret;
 }

@@ -2703,8 +2711,7 @@ static int reiserfs_write_end(struct fil
 	 ** transaction tracking stuff when the size changes.  So, we have
 	 ** to do the i_size updates here.
 	 */
-	pos += copied;
-	if (pos > inode->i_size) {
+	if (pos + copied > inode->i_size) {
 		struct reiserfs_transaction_handle myth;
 		reiserfs_write_lock(inode->i_sb);
 		/* If the file have grown beyond the border where it

@@ -2722,7 +2729,7 @@ static int reiserfs_write_end(struct fil
 			goto journal_error;
 		}
 		reiserfs_update_inode_transaction(inode);
-		inode->i_size = pos;
+		inode->i_size = pos + copied;
 		/*
 		 * this will just nest into our transaction.  It's important
 		 * to use mark_inode_dirty so the inode gets pushed around on the

@@ -2749,6 +2756,10 @@ static int reiserfs_write_end(struct fil
       out:
 	unlock_page(page);
 	page_cache_release(page);
+
+	if (pos + len > inode->i_size)
+		reiserfs_truncate_failed_write(inode);
+
 	return ret == 0 ? copied : ret;
 
       journal_error:

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

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

From: Erez Zadok <[email protected]>

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.

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

@@ -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)) {

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

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

From: Dan Carpenter <[email protected]>

commit ece550f51ba175c14ec3ec047815927d7386ea1f upstream.

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

---
 fs/ecryptfs/crypto.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -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;
 }
 

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

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

From: Greg Kroah-Hartman <[email protected]>

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.

---
 drivers/char/tty_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -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 {

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

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

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

Eric

Yes, that looks correct.

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

From: Greg Kroah-Hartman <[email protected]>
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.

index 97e01dc..5ef953e 100644
@@ -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,

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

Linus

Thanks, I'll go queue it up now.

greg k-h

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

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

From: Alan Stern <[email protected]>

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.

---
 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(-)

@@ -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]);

@@ -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]);
 		}

@@ -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. */

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

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

From: Alan Stern <[email protected]>

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.

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

@@ -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;
+			}
 		}
 	}
 

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

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

From: Alan Stern <[email protected]>

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.

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

@@ -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;

@@ -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;

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

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

From: Serge E. Hallyn <[email protected]>

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.

---
 ipc/msg.c |    1 +
 ipc/sem.c |    1 +
 ipc/shm.c |    1 +
 3 files changed, 3 insertions(+)

@@ -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
 

@@ -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
 

@@ -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
 

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

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

From: Christian Borntraeger <[email protected]>

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.

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

@@ -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)