All of lore.kernel.org
 help / color / mirror / Atom feed
* [ 00/21] 3.0.69-stable review
@ 2013-03-12 22:44 Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 01/21] ARM: VFP: fix emulation of second VFP instruction Greg Kroah-Hartman
                   ` (21 more replies)
  0 siblings, 22 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, torvalds, akpm, stable

This is the start of the stable review cycle for the 3.0.69 release.
There are 21 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 me know.

Responses should be made by Thu Mar 14 22:32:21 UTC 2013.
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/v3.0/stable-review/patch-3.0.69-rc1.gz
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Linux 3.0.69-rc1

Ben Hutchings <ben@decadent.org.uk>
    dmi_scan: fix missing check for _DMI_ signature in smbios_present()

Eric W. Biederman <ebiederm@xmission.com>
    decnet: Fix disappearing sysctl entries

Steven Rostedt <srostedt@redhat.com>
    ftrace: Update the kconfig for DYNAMIC_FTRACE

Tu, Xiaobing <xiaobing.tu@intel.com>
    Fix memory leak in cpufreq stats.

Al Viro <viro@ZenIV.linux.org.uk>
    vfs: fix pipe counter breakage

David Howells <dhowells@redhat.com>
    keys: fix race with concurrent install_user_keyrings()

Konstantin Khlebnikov <khlebnikov@openvz.org>
    e1000e: fix pci-device enable-counter balance

Takashi Iwai <tiwai@suse.de>
    ALSA: vmaster: Fix slave change notification

Sean Connor <sconnor004@allyinics.org>
    ALSA: ice1712: Initialize card->private_data properly

Alex Deucher <alexander.deucher@amd.com>
    drm/radeon: add primary dac adj quirk for R200 board

Mark Brown <broonie@opensource.wolfsonmicro.com>
    hwmon: (sht15) Check return value of regulator_enable()

NeilBrown <neilb@suse.de>
    md: raid0: fix error return from create_stripe_zones.

Felix Fietkau <nbd@openwrt.org>
    ath9k: fix RSSI dummy marker value

Rusty Russell <rusty@rustcorp.com.au>
    hw_random: make buffer usable in scatterlist.

Trond Myklebust <Trond.Myklebust@netapp.com>
    SUNRPC: Don't start the retransmission timer when out of socket space

Jeff Layton <jlayton@redhat.com>
    cifs: ensure that cifs_get_root() only traverses directories

Thomas Gleixner <tglx@linutronix.de>
    btrfs: Init io_lock after cloning btrfs device struct

Asias He <asias@redhat.com>
    target/pscsi: Fix page increment

Dan Carpenter <dan.carpenter@oracle.com>
    SCSI: dc395x: uninitialized variable in device_alloc()

Russell King <rmk+kernel@arm.linux.org.uk>
    ARM: fix scheduling while atomic warning in alignment handling code

Russell King <rmk+kernel@arm.linux.org.uk>
    ARM: VFP: fix emulation of second VFP instruction


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

Diffstat:

 Makefile                                |  4 ++--
 arch/arm/mm/alignment.c                 | 11 ++++-------
 arch/arm/vfp/vfpmodule.c                |  2 +-
 drivers/char/hw_random/core.c           | 19 ++++++++++++++++---
 drivers/cpufreq/cpufreq_stats.c         |  1 +
 drivers/firmware/dmi_scan.c             |  5 ++---
 drivers/gpu/drm/radeon/radeon_combios.c |  9 +++++++++
 drivers/hwmon/sht15.c                   |  8 +++++++-
 drivers/md/raid0.c                      |  2 +-
 drivers/net/e1000e/netdev.c             |  2 +-
 drivers/net/wireless/ath/ath9k/common.h |  2 +-
 drivers/scsi/dc395x.c                   |  2 +-
 drivers/target/target_core_pscsi.c      |  1 -
 fs/btrfs/volumes.c                      |  1 +
 fs/cifs/cifsfs.c                        |  5 +++++
 fs/pipe.c                               |  3 +++
 kernel/trace/Kconfig                    | 24 ++++++++++++++----------
 net/decnet/af_decnet.c                  |  4 ++++
 net/decnet/sysctl_net_decnet.c          | 28 ++++++++++++++++++++++++++++
 net/sunrpc/xprt.c                       |  6 +++++-
 security/keys/process_keys.c            |  2 +-
 sound/core/vmaster.c                    |  5 ++++-
 sound/pci/ice1712/ice1712.c             |  2 ++
 23 files changed, 113 insertions(+), 35 deletions(-)



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 01/21] ARM: VFP: fix emulation of second VFP instruction
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 02/21] ARM: fix scheduling while atomic warning in alignment handling code Greg Kroah-Hartman
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Martin Storsjö, Russell King

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Russell King <rmk+kernel@arm.linux.org.uk>

commit 5e4ba617c1b584b2e376f31a63bd4e734109318a upstream.

Martin Storsjö reports that the sequence:

        ee312ac1        vsub.f32        s4, s3, s2
        ee702ac0        vsub.f32        s5, s1, s0
        e59f0028        ldr             r0, [pc, #40]
        ee111a90        vmov            r1, s3

on Raspberry Pi (implementor 41 architecture 1 part 20 variant b rev 5)
where s3 is a denormal and s2 is zero results in incorrect behaviour -
the instruction "vsub.f32 s5, s1, s0" is not executed:

        VFP: bounce: trigger ee111a90 fpexc d0000780
        VFP: emulate: INST=0xee312ac1 SCR=0x00000000
        ...

As we can see, the instruction triggering the exception is the "vmov"
instruction, and we emulate the "vsub.f32 s4, s3, s2" but fail to
properly take account of the FPEXC_FP2V flag in FPEXC.  This is because
the test for the second instruction register being valid is bogus, and
will always skip emulation of the second instruction.

Reported-by: Martin Storsjö <martin@martin.st>
Tested-by: Martin Storsjö <martin@martin.st>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/arm/vfp/vfpmodule.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -369,7 +369,7 @@ void VFP_bounce(u32 trigger, u32 fpexc,
 	 * If there isn't a second FP instruction, exit now. Note that
 	 * the FPEXC.FP2V bit is valid only if FPEXC.EX is 1.
 	 */
-	if (fpexc ^ (FPEXC_EX | FPEXC_FP2V))
+	if ((fpexc & (FPEXC_EX | FPEXC_FP2V)) != (FPEXC_EX | FPEXC_FP2V))
 		goto exit;
 
 	/*



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 02/21] ARM: fix scheduling while atomic warning in alignment handling code
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 01/21] ARM: VFP: fix emulation of second VFP instruction Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 03/21] SCSI: dc395x: uninitialized variable in device_alloc() Greg Kroah-Hartman
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Paolo Pisati, Russell King

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Russell King <rmk+kernel@arm.linux.org.uk>

commit b255188f90e2bade1bd11a986dd1ca4861869f4d upstream.

Paolo Pisati reports that IPv6 triggers this warning:

BUG: scheduling while atomic: swapper/0/0/0x40000100
Modules linked in:
[<c001b1c4>] (unwind_backtrace+0x0/0xf0) from [<c0503c5c>] (__schedule_bug+0x48/0x5c)
[<c0503c5c>] (__schedule_bug+0x48/0x5c) from [<c0508608>] (__schedule+0x700/0x740)
[<c0508608>] (__schedule+0x700/0x740) from [<c007007c>] (__cond_resched+0x24/0x34)
[<c007007c>] (__cond_resched+0x24/0x34) from [<c05086dc>] (_cond_resched+0x3c/0x44)
[<c05086dc>] (_cond_resched+0x3c/0x44) from [<c0021f6c>] (do_alignment+0x178/0x78c)
[<c0021f6c>] (do_alignment+0x178/0x78c) from [<c00083e0>] (do_DataAbort+0x34/0x98)
[<c00083e0>] (do_DataAbort+0x34/0x98) from [<c0509a60>] (__dabt_svc+0x40/0x60)
Exception stack(0xc0763d70 to 0xc0763db8)
3d60:                                     e97e805e e97e806e 2c000000 11000000
3d80: ea86bb00 0000002c 00000011 e97e807e c076d2a8 e97e805e e97e806e 0000002c
3da0: 3d000000 c0763dbc c04b98fc c02a8490 00000113 ffffffff
[<c0509a60>] (__dabt_svc+0x40/0x60) from [<c02a8490>] (__csum_ipv6_magic+0x8/0xc8)

Fix this by using probe_kernel_address() stead of __get_user().

Reported-by: Paolo Pisati <p.pisati@gmail.com>
Tested-by: Paolo Pisati <p.pisati@gmail.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/arm/mm/alignment.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -721,7 +721,6 @@ do_alignment(unsigned long addr, unsigne
 	unsigned long instr = 0, instrptr;
 	int (*handler)(unsigned long addr, unsigned long instr, struct pt_regs *regs);
 	unsigned int type;
-	mm_segment_t fs;
 	unsigned int fault;
 	u16 tinstr = 0;
 	int isize = 4;
@@ -729,16 +728,15 @@ do_alignment(unsigned long addr, unsigne
 
 	instrptr = instruction_pointer(regs);
 
-	fs = get_fs();
-	set_fs(KERNEL_DS);
 	if (thumb_mode(regs)) {
-		fault = __get_user(tinstr, (u16 *)(instrptr & ~1));
+		u16 *ptr = (u16 *)(instrptr & ~1);
+		fault = probe_kernel_address(ptr, tinstr);
 		if (!fault) {
 			if (cpu_architecture() >= CPU_ARCH_ARMv7 &&
 			    IS_T32(tinstr)) {
 				/* Thumb-2 32-bit */
 				u16 tinst2 = 0;
-				fault = __get_user(tinst2, (u16 *)(instrptr+2));
+				fault = probe_kernel_address(ptr + 1, tinst2);
 				instr = (tinstr << 16) | tinst2;
 				thumb2_32b = 1;
 			} else {
@@ -747,8 +745,7 @@ do_alignment(unsigned long addr, unsigne
 			}
 		}
 	} else
-		fault = __get_user(instr, (u32 *)instrptr);
-	set_fs(fs);
+		fault = probe_kernel_address(instrptr, instr);
 
 	if (fault) {
 		type = TYPE_FAULT;



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 03/21] SCSI: dc395x: uninitialized variable in device_alloc()
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 01/21] ARM: VFP: fix emulation of second VFP instruction Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 02/21] ARM: fix scheduling while atomic warning in alignment handling code Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 04/21] target/pscsi: Fix page increment Greg Kroah-Hartman
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Dan Carpenter, Oliver Neukum,
	James Bottomley

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Dan Carpenter <dan.carpenter@oracle.com>

commit 208afec4f3be8c51ad6eebe6611dd6d2ad2fa298 upstream.

This bug was introduced back in bitkeeper days in 2003.  We use
"dcb->dev_mode" before it has been initialized.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Oliver Neukum <oliver@neukum.org>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3747,13 +3747,13 @@ static struct DeviceCtlBlk *device_alloc
 	dcb->max_command = 1;
 	dcb->target_id = target;
 	dcb->target_lun = lun;
+	dcb->dev_mode = eeprom->target[target].cfg0;
 #ifndef DC395x_NO_DISCONNECT
 	dcb->identify_msg =
 	    IDENTIFY(dcb->dev_mode & NTC_DO_DISCONNECT, lun);
 #else
 	dcb->identify_msg = IDENTIFY(0, lun);
 #endif
-	dcb->dev_mode = eeprom->target[target].cfg0;
 	dcb->inquiry7 = 0;
 	dcb->sync_mode = 0;
 	dcb->min_nego_period = clock_period[period_index];



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 04/21] target/pscsi: Fix page increment
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (2 preceding siblings ...)
  2013-03-12 22:44 ` [ 03/21] SCSI: dc395x: uninitialized variable in device_alloc() Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-16  2:10   ` Ben Hutchings
  2013-03-12 22:44 ` [ 05/21] btrfs: Init io_lock after cloning btrfs device struct Greg Kroah-Hartman
                   ` (17 subsequent siblings)
  21 siblings, 1 reply; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Asias He, Nicholas Bellinger

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Asias He <asias@redhat.com>

commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.

The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
address if the 'while (len > 0 && data_len > 0) { ... }' loop is
executed more than one once.

Signed-off-by: Asias He <asias@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/target/target_core_pscsi.c |    1 -
 1 file changed, 1 deletion(-)

--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
 				bio = NULL;
 			}
 
-			page++;
 			len -= bytes;
 			data_len -= bytes;
 			off = 0;



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 05/21] btrfs: Init io_lock after cloning btrfs device struct
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (3 preceding siblings ...)
  2013-03-12 22:44 ` [ 04/21] target/pscsi: Fix page increment Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 06/21] cifs: ensure that cifs_get_root() only traverses directories Greg Kroah-Hartman
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Chris Mason

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Thomas Gleixner <tglx@linutronix.de>

commit 1cba0cdf5e4dbcd9e5fa5b54d7a028e55e2ca057 upstream.

__btrfs_close_devices() clones btrfs device structs with
memcpy(). Some of the fields in the clone are reinitialized, but it's
missing to init io_lock. In mainline this goes unnoticed, but on RT it
leaves the plist pointing to the original about to be freed lock
struct.

Initialize io_lock after cloning, so no references to the original
struct are left.

Reported-and-tested-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chris Mason <chris.mason@fusionio.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/btrfs/volumes.c |    1 +
 1 file changed, 1 insertion(+)

--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -512,6 +512,7 @@ static int __btrfs_close_devices(struct
 		new_device->writeable = 0;
 		new_device->in_fs_metadata = 0;
 		new_device->can_discard = 0;
+		spin_lock_init(&new_device->io_lock);
 		list_replace_rcu(&device->dev_list, &new_device->dev_list);
 
 		call_rcu(&device->rcu, free_device);



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 06/21] cifs: ensure that cifs_get_root() only traverses directories
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (4 preceding siblings ...)
  2013-03-12 22:44 ` [ 05/21] btrfs: Init io_lock after cloning btrfs device struct Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 07/21] SUNRPC: Dont start the retransmission timer when out of socket space Greg Kroah-Hartman
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Jeff Layton, Steve French

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Jeff Layton <jlayton@redhat.com>

commit ce2ac52105aa663056dfc17966ebed1bf93e6e64 upstream.

Kjell Braden reported this oops:

[  833.211970] BUG: unable to handle kernel NULL pointer dereference at           (null)
[  833.212816] IP: [<          (null)>]           (null)
[  833.213280] PGD 1b9b2067 PUD e9f7067 PMD 0
[  833.213874] Oops: 0010 [#1] SMP
[  833.214344] CPU 0
[  833.214458] Modules linked in: des_generic md4 nls_utf8 cifs vboxvideo drm snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq bnep rfcomm snd_timer bluetooth snd_seq_device ppdev snd vboxguest parport_pc joydev mac_hid soundcore snd_page_alloc psmouse i2c_piix4 serio_raw lp parport usbhid hid e1000
[  833.215629]
[  833.215629] Pid: 1752, comm: mount.cifs Not tainted 3.0.0-rc7-bisectcifs-fec11dd9a0+ #18 innotek GmbH VirtualBox/VirtualBox
[  833.215629] RIP: 0010:[<0000000000000000>]  [<          (null)>]           (null)
[  833.215629] RSP: 0018:ffff8800119c9c50  EFLAGS: 00010282
[  833.215629] RAX: ffffffffa02186c0 RBX: ffff88000c427780 RCX: 0000000000000000
[  833.215629] RDX: 0000000000000000 RSI: ffff88000c427780 RDI: ffff88000c4362e8
[  833.215629] RBP: ffff8800119c9c88 R08: ffff88001fc15e30 R09: 00000000d69515c7
[  833.215629] R10: ffffffffa0201972 R11: ffff88000e8f6a28 R12: ffff88000c4362e8
[  833.215629] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88001181aaa6
[  833.215629] FS:  00007f2986171700(0000) GS:ffff88001fc00000(0000) knlGS:0000000000000000
[  833.215629] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  833.215629] CR2: 0000000000000000 CR3: 000000001b982000 CR4: 00000000000006f0
[  833.215629] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  833.215629] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  833.215629] Process mount.cifs (pid: 1752, threadinfo ffff8800119c8000, task ffff88001c1c16f0)
[  833.215629] Stack:
[  833.215629]  ffffffff8116a9b5 ffff8800119c9c88 ffffffff81178075 0000000000000286
[  833.215629]  0000000000000000 ffff88000c4276c0 ffff8800119c9ce8 ffff8800119c9cc8
[  833.215629]  ffffffff8116b06e ffff88001bc6fc00 ffff88000c4276c0 ffff88000c4276c0
[  833.215629] Call Trace:
[  833.215629]  [<ffffffff8116a9b5>] ? d_alloc_and_lookup+0x45/0x90
[  833.215629]  [<ffffffff81178075>] ? d_lookup+0x35/0x60
[  833.215629]  [<ffffffff8116b06e>] __lookup_hash.part.14+0x9e/0xc0
[  833.215629]  [<ffffffff8116b1d6>] lookup_one_len+0x146/0x1e0
[  833.215629]  [<ffffffff815e4f7e>] ? _raw_spin_lock+0xe/0x20
[  833.215629]  [<ffffffffa01eef0d>] cifs_do_mount+0x26d/0x500 [cifs]
[  833.215629]  [<ffffffff81163bd3>] mount_fs+0x43/0x1b0
[  833.215629]  [<ffffffff8117d41a>] vfs_kern_mount+0x6a/0xd0
[  833.215629]  [<ffffffff8117e584>] do_kern_mount+0x54/0x110
[  833.215629]  [<ffffffff8117fdc2>] do_mount+0x262/0x840
[  833.215629]  [<ffffffff81108a0e>] ? __get_free_pages+0xe/0x50
[  833.215629]  [<ffffffff8117f9ca>] ? copy_mount_options+0x3a/0x180
[  833.215629]  [<ffffffff8118075d>] sys_mount+0x8d/0xe0
[  833.215629]  [<ffffffff815ece82>] system_call_fastpath+0x16/0x1b
[  833.215629] Code:  Bad RIP value.
[  833.215629] RIP  [<          (null)>]           (null)
[  833.215629]  RSP <ffff8800119c9c50>
[  833.215629] CR2: 0000000000000000
[  833.238525] ---[ end trace ec00758b8d44f529 ]---

When walking down the path on the server, it's possible to hit a
symlink. The path walking code assumes that the caller will handle that
situation properly, but cifs_get_root() isn't set up for it. This patch
prevents the oops by simply returning an error.

A better solution would be to try and chase the symlinks here, but that's
fairly complicated to handle.

Fixes:

    https://bugzilla.kernel.org/show_bug.cgi?id=53221

Reported-and-tested-by: Kjell Braden <afflux@pentabarf.de>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/cifs/cifsfs.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -571,6 +571,11 @@ cifs_get_root(struct smb_vol *vol, struc
 			dentry = ERR_PTR(-ENOENT);
 			break;
 		}
+		if (!S_ISDIR(dir->i_mode)) {
+			dput(dentry);
+			dentry = ERR_PTR(-ENOTDIR);
+			break;
+		}
 
 		/* skip separators */
 		while (*s == sep)



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 07/21] SUNRPC: Dont start the retransmission timer when out of socket space
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (5 preceding siblings ...)
  2013-03-12 22:44 ` [ 06/21] cifs: ensure that cifs_get_root() only traverses directories Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 08/21] hw_random: make buffer usable in scatterlist Greg Kroah-Hartman
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Trond Myklebust

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Trond Myklebust <Trond.Myklebust@netapp.com>

commit a9a6b52ee1baa865283a91eb8d443ee91adfca56 upstream.

If the socket is full, we're better off just waiting until it empties,
or until the connection is broken. The reason why we generally don't
want to time out is that the call to xprt->ops->release_xprt() will
trigger a connection reset, which isn't helpful...

Let's make an exception for soft RPC calls, since they have to provide
timeout guarantees.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/sunrpc/xprt.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -471,13 +471,17 @@ EXPORT_SYMBOL_GPL(xprt_wake_pending_task
  * xprt_wait_for_buffer_space - wait for transport output buffer to clear
  * @task: task to be put to sleep
  * @action: function pointer to be executed after wait
+ *
+ * Note that we only set the timer for the case of RPC_IS_SOFT(), since
+ * we don't in general want to force a socket disconnection due to
+ * an incomplete RPC call transmission.
  */
 void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action)
 {
 	struct rpc_rqst *req = task->tk_rqstp;
 	struct rpc_xprt *xprt = req->rq_xprt;
 
-	task->tk_timeout = req->rq_timeout;
+	task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0;
 	rpc_sleep_on(&xprt->pending, task, action);
 }
 EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space);



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 08/21] hw_random: make buffer usable in scatterlist.
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (6 preceding siblings ...)
  2013-03-12 22:44 ` [ 07/21] SUNRPC: Dont start the retransmission timer when out of socket space Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 09/21] ath9k: fix RSSI dummy marker value Greg Kroah-Hartman
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Aurelien Jarno, Rusty Russell

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Rusty Russell <rusty@rustcorp.com.au>

commit f7f154f1246ccc5a0a7e9ce50932627d60a0c878 upstream.

virtio_rng feeds the randomness buffer handed by the core directly
into the scatterlist, since commit bb347d98079a547e80bd4722dee1de61e4dca0e8.

However, if CONFIG_HW_RANDOM=m, the static buffer isn't a linear address
(at least on most archs).  We could fix this in virtio_rng, but it's actually
far easier to just do it in the core as virtio_rng would have to allocate
a buffer every time (it doesn't know how much the core will want to read).

Reported-by: Aurelien Jarno <aurelien@aurel32.net>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/char/hw_random/core.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -40,6 +40,7 @@
 #include <linux/init.h>
 #include <linux/miscdevice.h>
 #include <linux/delay.h>
+#include <linux/slab.h>
 #include <asm/uaccess.h>
 
 
@@ -52,8 +53,12 @@ static struct hwrng *current_rng;
 static LIST_HEAD(rng_list);
 static DEFINE_MUTEX(rng_mutex);
 static int data_avail;
-static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES]
-	__cacheline_aligned;
+static u8 *rng_buffer;
+
+static size_t rng_buffer_size(void)
+{
+	return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
+}
 
 static inline int hwrng_init(struct hwrng *rng)
 {
@@ -116,7 +121,7 @@ static ssize_t rng_dev_read(struct file
 
 		if (!data_avail) {
 			bytes_read = rng_get_data(current_rng, rng_buffer,
-				sizeof(rng_buffer),
+				rng_buffer_size(),
 				!(filp->f_flags & O_NONBLOCK));
 			if (bytes_read < 0) {
 				err = bytes_read;
@@ -307,6 +312,14 @@ int hwrng_register(struct hwrng *rng)
 
 	mutex_lock(&rng_mutex);
 
+	/* kmalloc makes this safe for virt_to_page() in virtio_rng.c */
+	err = -ENOMEM;
+	if (!rng_buffer) {
+		rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL);
+		if (!rng_buffer)
+			goto out_unlock;
+	}
+
 	/* Must not register two RNGs with the same name. */
 	err = -EEXIST;
 	list_for_each_entry(tmp, &rng_list, list) {



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 09/21] ath9k: fix RSSI dummy marker value
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (7 preceding siblings ...)
  2013-03-12 22:44 ` [ 08/21] hw_random: make buffer usable in scatterlist Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 10/21] md: raid0: fix error return from create_stripe_zones Greg Kroah-Hartman
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Felix Fietkau, John W. Linville

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Felix Fietkau <nbd@openwrt.org>

commit a3d63cadbad97671d740a9698acc2c95d1ca6e79 upstream.

RSSI is being stored internally as s8 in several places. The indication
of an unset RSSI value, ATH_RSSI_DUMMY_MARKER, was supposed to have been
set to 127, but ended up being set to 0x127 because of a code cleanup
mistake. This could lead to invalid signal strength values in a few
places.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/net/wireless/ath/ath9k/common.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/ath/ath9k/common.h
+++ b/drivers/net/wireless/ath/ath9k/common.h
@@ -35,7 +35,7 @@
 #define WME_AC_BK   3
 #define WME_NUM_AC  4
 
-#define ATH_RSSI_DUMMY_MARKER   0x127
+#define ATH_RSSI_DUMMY_MARKER   127
 #define ATH_RSSI_LPF_LEN 		10
 #define RSSI_LPF_THRESHOLD		-20
 #define ATH_RSSI_EP_MULTIPLIER     (1<<7)



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 10/21] md: raid0: fix error return from create_stripe_zones.
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (8 preceding siblings ...)
  2013-03-12 22:44 ` [ 09/21] ath9k: fix RSSI dummy marker value Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 11/21] hwmon: (sht15) Check return value of regulator_enable() Greg Kroah-Hartman
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, NeilBrown

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: NeilBrown <neilb@suse.de>

commit 58ebb34c49fcfcaa029e4b1c1453d92583900f9a upstream.

Create_stripe_zones returns an error slightly differently to
raid0_run and to raid0_takeover_*.

The error returned used by the second was wrong and an error would
result in mddev->private being set to NULL and sooner or later a
crash.

So never return NULL, return ERR_PTR(err), not NULL from
create_stripe_zones.

This bug has been present since 2.6.35 so the fix is suitable
for any kernel since then.

Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -283,7 +283,7 @@ abort:
 	kfree(conf->strip_zone);
 	kfree(conf->devlist);
 	kfree(conf);
-	*private_conf = NULL;
+	*private_conf = ERR_PTR(err);
 	return err;
 }
 



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 11/21] hwmon: (sht15) Check return value of regulator_enable()
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (9 preceding siblings ...)
  2013-03-12 22:44 ` [ 10/21] md: raid0: fix error return from create_stripe_zones Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-16  4:15   ` Ben Hutchings
  2013-03-12 22:44 ` [ 12/21] drm/radeon: add primary dac adj quirk for R200 board Greg Kroah-Hartman
                   ` (10 subsequent siblings)
  21 siblings, 1 reply; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Mark Brown, Guenter Roeck

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Mark Brown <broonie@opensource.wolfsonmicro.com>

commit 3e78080f81481aa8340374d5a37ae033c1cf4272 upstream.

Not having power is a pretty serious error so check that we are able to
enable the supply and error out if we can't.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

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

--- a/drivers/hwmon/sht15.c
+++ b/drivers/hwmon/sht15.c
@@ -926,7 +926,13 @@ static int __devinit sht15_probe(struct
 		if (voltage)
 			data->supply_uV = voltage;
 
-		regulator_enable(data->reg);
+		ret = regulator_enable(data->reg);
+		if (ret != 0) {
+			dev_err(&pdev->dev,
+				"failed to enable regulator: %d\n", ret);
+			return ret;
+		}
+
 		/*
 		 * Setup a notifier block to update this if another device
 		 * causes the voltage to change



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 12/21] drm/radeon: add primary dac adj quirk for R200 board
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (10 preceding siblings ...)
  2013-03-12 22:44 ` [ 11/21] hwmon: (sht15) Check return value of regulator_enable() Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 13/21] ALSA: ice1712: Initialize card->private_data properly Greg Kroah-Hartman
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Alex Deucher

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Alex Deucher <alexander.deucher@amd.com>

commit e8fc41377f5037ff7a661ea06adc05f1daec1548 upstream.

vbios values are wrong leading to colors that are
too bright.  Use the default values instead.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/gpu/drm/radeon/radeon_combios.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/drivers/gpu/drm/radeon/radeon_combios.c
+++ b/drivers/gpu/drm/radeon/radeon_combios.c
@@ -958,6 +958,15 @@ struct radeon_encoder_primary_dac *radeo
 			found = 1;
 	}
 
+	/* quirks */
+	/* Radeon 9100 (R200) */
+	if ((dev->pdev->device == 0x514D) &&
+	    (dev->pdev->subsystem_vendor == 0x174B) &&
+	    (dev->pdev->subsystem_device == 0x7149)) {
+		/* vbios value is bad, use the default */
+		found = 0;
+	}
+
 	if (!found) /* fallback to defaults */
 		radeon_legacy_get_primary_dac_info_from_table(rdev, p_dac);
 



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 13/21] ALSA: ice1712: Initialize card->private_data properly
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (11 preceding siblings ...)
  2013-03-12 22:44 ` [ 12/21] drm/radeon: add primary dac adj quirk for R200 board Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 14/21] ALSA: vmaster: Fix slave change notification Greg Kroah-Hartman
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Sean Connor, Takashi Iwai

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Sean Connor <sconnor004@allyinics.org>

commit 69a4cfdd444d1fe5c24d29b3a063964ac165d2cd upstream.

Set card->private_data in snd_ice1712_create for fixing NULL
dereference in snd_ice1712_remove().

Signed-off-by: Sean Connor <sconnor004@allyinics.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 sound/pci/ice1712/ice1712.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/sound/pci/ice1712/ice1712.c
+++ b/sound/pci/ice1712/ice1712.c
@@ -2595,6 +2595,8 @@ static int __devinit snd_ice1712_create(
 	snd_ice1712_proc_init(ice);
 	synchronize_irq(pci->irq);
 
+	card->private_data = ice;
+
 	err = pci_request_regions(pci, "ICE1712");
 	if (err < 0) {
 		kfree(ice);



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 14/21] ALSA: vmaster: Fix slave change notification
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (12 preceding siblings ...)
  2013-03-12 22:44 ` [ 13/21] ALSA: ice1712: Initialize card->private_data properly Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 15/21] e1000e: fix pci-device enable-counter balance Greg Kroah-Hartman
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Takashi Iwai

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Takashi Iwai <tiwai@suse.de>

commit 2069d483b39a603a5f3428a19d3b4ac89aa97f48 upstream.

When a value of a vmaster slave control is changed, the ctl change
notification is sometimes ignored.  This happens when the master
control overrides, e.g. when the corresponding master control is
muted.  The reason is that slave_put() returns the value of the actual
slave put callback, and it doesn't reflect the virtual slave value
change.

This patch fixes the function just to return 1 whenever a slave value
is changed.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 sound/core/vmaster.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -207,7 +207,10 @@ static int slave_put(struct snd_kcontrol
 	}
 	if (!changed)
 		return 0;
-	return slave_put_val(slave, ucontrol);
+	err = slave_put_val(slave, ucontrol);
+	if (err < 0)
+		return err;
+	return 1;
 }
 
 static int slave_tlv_cmd(struct snd_kcontrol *kcontrol,



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 15/21] e1000e: fix pci-device enable-counter balance
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (13 preceding siblings ...)
  2013-03-12 22:44 ` [ 14/21] ALSA: vmaster: Fix slave change notification Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 16/21] keys: fix race with concurrent install_user_keyrings() Greg Kroah-Hartman
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Konstantin Khlebnikov, Bruce Allan,
	Rafael J. Wysocki, Borislav Petkov, Aaron Brown, Jeff Kirsher

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Konstantin Khlebnikov <khlebnikov@openvz.org>

commit 4e0855dff094b0d56d6b5b271e0ce7851cc1e063 upstream.

This patch removes redundant and unbalanced pci_disable_device() from
__e1000_shutdown(). pci_clear_master() is enough, device can go into
suspended state with elevated enable_cnt.

Bug was introduced in commit 23606cf5d1192c2b17912cb2ef6e62f9b11de133
("e1000e / PCI / PM: Add basic runtime PM support (rev. 4)") in v2.6.35

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Borislav Petkov <bp@suse.de>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/net/e1000e/netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -5330,7 +5330,7 @@ static int __e1000_shutdown(struct pci_d
 	 */
 	e1000e_release_hw_control(adapter);
 
-	pci_disable_device(pdev);
+	pci_clear_master(pdev);
 
 	return 0;
 }



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 16/21] keys: fix race with concurrent install_user_keyrings()
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (14 preceding siblings ...)
  2013-03-12 22:44 ` [ 15/21] e1000e: fix pci-device enable-counter balance Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 17/21] vfs: fix pipe counter breakage Greg Kroah-Hartman
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, David Howells, Mateusz Guzik,
	Andrew Morton, James Morris

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: David Howells <dhowells@redhat.com>

commit 0da9dfdd2cd9889201bc6f6f43580c99165cd087 upstream.

This fixes CVE-2013-1792.

There is a race in install_user_keyrings() that can cause a NULL pointer
dereference when called concurrently for the same user if the uid and
uid-session keyrings are not yet created.  It might be possible for an
unprivileged user to trigger this by calling keyctl() from userspace in
parallel immediately after logging in.

Assume that we have two threads both executing lookup_user_key(), both
looking for KEY_SPEC_USER_SESSION_KEYRING.

	THREAD A			THREAD B
	===============================	===============================
					==>call install_user_keyrings();
	if (!cred->user->session_keyring)
	==>call install_user_keyrings()
					...
					user->uid_keyring = uid_keyring;
	if (user->uid_keyring)
		return 0;
	<==
	key = cred->user->session_keyring [== NULL]
					user->session_keyring = session_keyring;
	atomic_inc(&key->usage); [oops]

At the point thread A dereferences cred->user->session_keyring, thread B
hasn't updated user->session_keyring yet, but thread A assumes it is
populated because install_user_keyrings() returned ok.

The race window is really small but can be exploited if, for example,
thread B is interrupted or preempted after initializing uid_keyring, but
before doing setting session_keyring.

This couldn't be reproduced on a stock kernel.  However, after placing
systemtap probe on 'user->session_keyring = session_keyring;' that
introduced some delay, the kernel could be crashed reliably.

Fix this by checking both pointers before deciding whether to return.
Alternatively, the test could be done away with entirely as it is checked
inside the mutex - but since the mutex is global, that may not be the best
way.

Signed-off-by: David Howells <dhowells@redhat.com>
Reported-by: Mateusz Guzik <mguzik@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 security/keys/process_keys.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -54,7 +54,7 @@ int install_user_keyrings(void)
 
 	kenter("%p{%u}", user, user->uid);
 
-	if (user->uid_keyring) {
+	if (user->uid_keyring && user->session_keyring) {
 		kleave(" = 0 [exist]");
 		return 0;
 	}



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 17/21] vfs: fix pipe counter breakage
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (15 preceding siblings ...)
  2013-03-12 22:44 ` [ 16/21] keys: fix race with concurrent install_user_keyrings() Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 18/21] Fix memory leak in cpufreq stats Greg Kroah-Hartman
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Dave Jones, Linus Torvalds

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Al Viro <viro@ZenIV.linux.org.uk>

commit a930d8790552658140d7d0d2e316af4f0d76a512 upstream.

If you open a pipe for neither read nor write, the pipe code will not
add any usage counters to the pipe, causing the 'struct pipe_inode_info"
to be potentially released early.

That doesn't normally matter, since you cannot actually use the pipe,
but the pipe release code - particularly fasync handling - still expects
the actual pipe infrastructure to all be there.  And rather than adding
NULL pointer checks, let's just disallow this case, the same way we
already do for the named pipe ("fifo") case.

This is ancient going back to pre-2.4 days, and until trinity, nobody
naver noticed.

Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/pipe.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -859,6 +859,9 @@ pipe_rdwr_open(struct inode *inode, stru
 {
 	int ret = -ENOENT;
 
+	if (!(filp->f_mode & (FMODE_READ|FMODE_WRITE)))
+		return -EINVAL;
+
 	mutex_lock(&inode->i_mutex);
 
 	if (inode->i_pipe) {



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 18/21] Fix memory leak in cpufreq stats.
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (16 preceding siblings ...)
  2013-03-12 22:44 ` [ 17/21] vfs: fix pipe counter breakage Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 19/21] ftrace: Update the kconfig for DYNAMIC_FTRACE Greg Kroah-Hartman
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, xiaobing tu, guifang tang,
	Rafael J. Wysocki, Colin Cross

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: "Tu, Xiaobing" <xiaobing.tu@intel.com>

commit e37736777254ce1abc85493a5cacbefe5983b896 upstream.

When system enters sleep, non-boot CPUs will be disabled.
Cpufreq stats sysfs is created when the CPU is up, but it is not
freed when the CPU is going down. This will cause memory leak.

Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
Signed-off-by: guifang tang <guifang.tang@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Colin Cross <ccross@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/cpufreq/cpufreq_stats.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -329,6 +329,7 @@ static int __cpuinit cpufreq_stat_cpu_ca
 		cpufreq_update_policy(cpu);
 		break;
 	case CPU_DOWN_PREPARE:
+	case CPU_DOWN_PREPARE_FROZEN:
 		cpufreq_stats_free_sysfs(cpu);
 		break;
 	case CPU_DEAD:



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 19/21] ftrace: Update the kconfig for DYNAMIC_FTRACE
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (17 preceding siblings ...)
  2013-03-12 22:44 ` [ 18/21] Fix memory leak in cpufreq stats Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-12 22:44 ` [ 20/21] decnet: Fix disappearing sysctl entries Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, stable, Ezequiel Garcia, Steven Rostedt

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Steven Rostedt <srostedt@redhat.com>

commit db05021d49a994ee40a9735d9c3cb0060c9babb8 upstream.

The prompt to enable DYNAMIC_FTRACE (the ability to nop and
enable function tracing at run time) had a confusing statement:

 "enable/disable ftrace tracepoints dynamically"

This was written before tracepoints were added to the kernel,
but now that tracepoints have been added, this is very confusing
and has confused people enough to give wrong information during
presentations.

Not only that, I looked at the help text, and it still references
that dreaded daemon that use to wake up once a second to update
the nop locations and brick NICs, that hasn't been around for over
five years.

Time to bring the text up to the current decade.

Reported-by: Ezequiel Garcia <elezegarcia@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 kernel/trace/Kconfig |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -386,24 +386,28 @@ config KPROBE_EVENT
 	  If you want to use perf tools, this option is strongly recommended.
 
 config DYNAMIC_FTRACE
-	bool "enable/disable ftrace tracepoints dynamically"
+	bool "enable/disable function tracing dynamically"
 	depends on FUNCTION_TRACER
 	depends on HAVE_DYNAMIC_FTRACE
 	default y
 	help
-          This option will modify all the calls to ftrace dynamically
-	  (will patch them out of the binary image and replace them
-	  with a No-Op instruction) as they are called. A table is
-	  created to dynamically enable them again.
+	  This option will modify all the calls to function tracing
+	  dynamically (will patch them out of the binary image and
+	  replace them with a No-Op instruction) on boot up. During
+	  compile time, a table is made of all the locations that ftrace
+	  can function trace, and this table is linked into the kernel
+	  image. When this is enabled, functions can be individually
+	  enabled, and the functions not enabled will not affect
+	  performance of the system.
+
+	  See the files in /sys/kernel/debug/tracing:
+	    available_filter_functions
+	    set_ftrace_filter
+	    set_ftrace_notrace
 
 	  This way a CONFIG_FUNCTION_TRACER kernel is slightly larger, but
 	  otherwise has native performance as long as no tracing is active.
 
-	  The changes to the code are done by a kernel thread that
-	  wakes up once a second and checks to see if any ftrace calls
-	  were made. If so, it runs stop_machine (stops all CPUS)
-	  and modifies the code to jump over the call to ftrace.
-
 config FUNCTION_PROFILER
 	bool "Kernel function profiler"
 	depends on FUNCTION_TRACER



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 20/21] decnet: Fix disappearing sysctl entries
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (18 preceding siblings ...)
  2013-03-12 22:44 ` [ 19/21] ftrace: Update the kconfig for DYNAMIC_FTRACE Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
       [not found]   ` <B401117D-23F6-4911-8D72-344EE1A022F6@usgs.gov>
  2013-03-12 22:44 ` [ 21/21] dmi_scan: fix missing check for _DMI_ signature in smbios_present() Greg Kroah-Hartman
  2013-03-13  3:56 ` [ 00/21] 3.0.69-stable review Shuah Khan
  21 siblings, 1 reply; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Larry Baker, Eric W. Biederman, David Miller

3.0-stable review patch.  If anyone has any objections, please let me know.

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


When decnet is built as a module a simple:
echo 0.0 >/proc/sys/net/decnet/node_address

results in most of the sysctl entries under /proc/sys/net/decnet and
/proc/sys/net/decnet/conf disappearing.

For more details see http://www.spinics.net/lists/netdev/msg226123.html.

This change applies the same workaround used in
net/core/sysctl_net_core.c and net/ipv6/sysctl_net_ipv6.c of creating
a skeleton of decnet sysctl entries before doing anything else.

The problem first appeared in kernel 2.6.27.  The later rewrite of
sysctl in kernel 3.4 restored the previous behavior and eliminated the
need for this workaround.

This patch was heavily inspired by a similar but more complex patch by
Larry Baker.

Reported-by: Larry Baker <baker@usgs.gov>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Acked-by: David Miller <davem@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/decnet/af_decnet.c         |    4 ++++
 net/decnet/sysctl_net_decnet.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -2355,6 +2355,8 @@ static const struct proto_ops dn_proto_o
 	.sendpage =	sock_no_sendpage,
 };
 
+void dn_register_sysctl_skeleton(void);
+void dn_unregister_sysctl_skeleton(void);
 void dn_register_sysctl(void);
 void dn_unregister_sysctl(void);
 
@@ -2375,6 +2377,7 @@ static int __init decnet_init(void)
 	if (rc != 0)
 		goto out;
 
+	dn_register_sysctl_skeleton();
 	dn_neigh_init();
 	dn_dev_init();
 	dn_route_init();
@@ -2414,6 +2417,7 @@ static void __exit decnet_exit(void)
 	dn_fib_cleanup();
 
 	proc_net_remove(&init_net, "decnet");
+	dn_unregister_sysctl_skeleton();
 
 	proto_unregister(&dn_proto);
 
--- a/net/decnet/sysctl_net_decnet.c
+++ b/net/decnet/sysctl_net_decnet.c
@@ -55,6 +55,7 @@ static int max_decnet_no_fc_max_cwnd[] =
 static char node_name[7] = "???";
 
 static struct ctl_table_header *dn_table_header = NULL;
+static struct ctl_table_header *dn_skeleton_table_header = NULL;
 
 /*
  * ctype.h :-)
@@ -356,6 +357,27 @@ static struct ctl_path dn_path[] = {
 	{ }
 };
 
+static struct ctl_table empty[1];
+
+static struct ctl_table dn_skeleton[] = {
+	{
+		.procname = "conf",
+		.mode = 0555,
+		.child = empty,
+	},
+	{ }
+};
+
+void dn_register_sysctl_skeleton(void)
+{
+	dn_skeleton_table_header = register_sysctl_paths(dn_path, dn_skeleton);
+}
+
+void dn_unregister_sysctl_skeleton(void)
+{
+	unregister_sysctl_table(dn_skeleton_table_header);
+}
+
 void dn_register_sysctl(void)
 {
 	dn_table_header = register_sysctl_paths(dn_path, dn_table);
@@ -367,6 +389,12 @@ void dn_unregister_sysctl(void)
 }
 
 #else  /* CONFIG_SYSCTL */
+void dn_register_sysctl_skeleton(void)
+{
+}
+void dn_unregister_sysctl_skeleton(void)
+{
+}
 void dn_unregister_sysctl(void)
 {
 }



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [ 21/21] dmi_scan: fix missing check for _DMI_ signature in smbios_present()
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (19 preceding siblings ...)
  2013-03-12 22:44 ` [ 20/21] decnet: Fix disappearing sysctl entries Greg Kroah-Hartman
@ 2013-03-12 22:44 ` Greg Kroah-Hartman
  2013-03-13  3:56 ` [ 00/21] 3.0.69-stable review Shuah Khan
  21 siblings, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 22:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Ben Hutchings, Tim McGrath,
	Zhenzhong Duan, Andrew Morton, Linus Torvalds

3.0-stable review patch.  If anyone has any objections, please let me know.

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

From: Ben Hutchings <ben@decadent.org.uk>

commit a40e7cf8f06b4e322ba902e4e9f6a6b0c2daa907 upstream.

Commit 9f9c9cbb6057 ("drivers/firmware/dmi_scan.c: fetch dmi version
from SMBIOS if it exists") hoisted the check for "_DMI_" into
dmi_scan_machine(), which means that we don't bother to check for
"_DMI_" at offset 16 in an SMBIOS entry.  smbios_present() may also call
dmi_present() for an address where we found "_SM_", if it failed further
validation.

Check for "_DMI_" in smbios_present() before calling dmi_present().

[akpm@linux-foundation.org: fix build]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Reported-by: Tim McGrath <tmhikaru@gmail.com>
Tested-by: Tim Mcgrath <tmhikaru@gmail.com>
Cc: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/firmware/dmi_scan.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -442,7 +442,6 @@ static int __init dmi_present(const char
 static int __init smbios_present(const char __iomem *p)
 {
 	u8 buf[32];
-	int offset = 0;
 
 	memcpy_fromio(buf, p, 32);
 	if ((buf[5] < 32) && dmi_checksum(buf, buf[5])) {
@@ -461,9 +460,9 @@ static int __init smbios_present(const c
 			dmi_ver = 0x0206;
 			break;
 		}
-		offset = 16;
+		return memcmp(p + 16, "_DMI_", 5) || dmi_present(p + 16);
 	}
-	return dmi_present(buf + offset);
+	return 1;
 }
 
 void __init dmi_scan_machine(void)



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 20/21] decnet: Fix disappearing sysctl entries
       [not found]   ` <B401117D-23F6-4911-8D72-344EE1A022F6@usgs.gov>
@ 2013-03-12 23:04     ` Eric W. Biederman
       [not found]       ` <3EDA829E-3898-4626-9C17-CCE31E8C0554@usgs.gov>
  2013-03-12 23:06     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 35+ messages in thread
From: Eric W. Biederman @ 2013-03-12 23:04 UTC (permalink / raw)
  To: Larry Baker; +Cc: Greg Kroah-Hartman, linux-kernel, stable, David Miller

Larry Baker <baker@usgs.gov> writes:

> Greg,
>
> Will there also be a 2.6-stable patch (Eric's first submission)?  My CentOS 6.3
> systems still use 2.6 kernels.

Larry what is in CentOS is for the CentOS maintainers to determine.
Last I checked they followed RHEL and RHEL seems to have so little
interest in decnet that the module isn't even built.

Pushing it to the patch to -stable makes the patch available to them if
they want to pick up the bugfix.

Beyond that if you look at
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/log/?id=refs/tags/v2.6.32.60
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/log/?id=refs/tags/v2.6.34.14

You can see that it is Willy Tarreau that makes releases for 2.6.32.y
and it is Paul Gortmaker that make releases releases for 2.6.34.y

So I don't expect Greg KH has anything to do with those kernels anymore.

Eric

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 20/21] decnet: Fix disappearing sysctl entries
       [not found]   ` <B401117D-23F6-4911-8D72-344EE1A022F6@usgs.gov>
  2013-03-12 23:04     ` Eric W. Biederman
@ 2013-03-12 23:06     ` Greg Kroah-Hartman
  1 sibling, 0 replies; 35+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 23:06 UTC (permalink / raw)
  To: Larry Baker; +Cc: linux-kernel, stable, Eric W. Biederman, David Miller

On Tue, Mar 12, 2013 at 03:52:19PM -0700, Larry Baker wrote:
> Greg,
> 
> Will there also be a 2.6-stable patch (Eric's first submission)?  My CentOS 6.3
> systems still use 2.6 kernels.

I do not maintain any 2.6-stable releases, sorry.  I suggest you work
with Centos to get this patch into their kernel as soon as possible, as
I do not know when the next 2.6-stable kernel will be released.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 00/21] 3.0.69-stable review
  2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
                   ` (20 preceding siblings ...)
  2013-03-12 22:44 ` [ 21/21] dmi_scan: fix missing check for _DMI_ signature in smbios_present() Greg Kroah-Hartman
@ 2013-03-13  3:56 ` Shuah Khan
  21 siblings, 0 replies; 35+ messages in thread
From: Shuah Khan @ 2013-03-13  3:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, torvalds, akpm, stable

On Tue, Mar 12, 2013 at 4:44 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> This is the start of the stable review cycle for the 3.0.69 release.
> There are 21 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 me know.
>
> Responses should be made by Thu Mar 14 22:32:21 UTC 2013.
> 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/v3.0/stable-review/patch-3.0.69-rc1.gz
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h
>

Patches applied cleanly to 3.0.68, 3.4.35, and 3.8.2

Compiled and booted on the following systems:

HP EliteBook 6930p Intel(R) Core(TM)2 Duo CPU T9400 @ 2.53GHz
HP ProBook 6475b AMD A10-4600M APU with Radeon(tm) HD Graphics

dmesgs for all releases look good. No regressions compared to the previous
dmesgs for each of these releases.

Cross-compile tests results:

alpha: defconfig passed on all
arm: defconfig passed on all
arm64: not applicable to 3.0.y, 3.4.y. defconfig passed on 3.8.y
c6x: not applicable to 3.0.y, defconfig passed on 3.4.y, and 3.8.y.
mips: defconfig passed on all
mipsel: defconfig passed on all
powerpc: wii_defconfig passed on all
sh: defconfig passed on all
sparc: defconfig passed on all
tile: tilegx_defconfig passed on all

-- Shuah

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 20/21] decnet: Fix disappearing sysctl entries
       [not found]       ` <3EDA829E-3898-4626-9C17-CCE31E8C0554@usgs.gov>
@ 2013-03-13 20:05         ` Eric W. Biederman
  0 siblings, 0 replies; 35+ messages in thread
From: Eric W. Biederman @ 2013-03-13 20:05 UTC (permalink / raw)
  To: Larry Baker; +Cc: Greg Kroah-Hartman, linux-kernel, stable, David Miller

Larry Baker <baker@usgs.gov> writes:

> I gather then that the long term kernels shown at https://www.kernel.org/ might eventually get updated if Willy Tarreau and Paul Gortmaker subscribe to the stable@vger.kernel.org list and decide to apply Eric's patches to the mainline stable 2.6 kernels.  Otherwise (or, in addition?), I would have to file bug reports to, e.g., CentOS, for them to update their distributions.  I can refer CentOS to http://www.spinics.net/lists/netdev/msg226123.html for that.  In the mean time, do I have your permission Eric to post your patches to the Linux DECnet web site?

Of course.  The patch is covered under the GPL license which gives
anyone the right to redistribute it.

Eric

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-12 22:44 ` [ 04/21] target/pscsi: Fix page increment Greg Kroah-Hartman
@ 2013-03-16  2:10   ` Ben Hutchings
  2013-03-18  5:35     ` Asias He
  0 siblings, 1 reply; 35+ messages in thread
From: Ben Hutchings @ 2013-03-16  2:10 UTC (permalink / raw)
  To: Asias He, Nicholas Bellinger; +Cc: linux-kernel, stable, Greg Kroah-Hartman

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> 3.0-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Asias He <asias@redhat.com>
> 
> commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> 
> The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> executed more than one once.
> 
> Signed-off-by: Asias He <asias@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  drivers/target/target_core_pscsi.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
>  				bio = NULL;
>  			}
>  
> -			page++;
>  			len -= bytes;
>  			data_len -= bytes;
>  			off = 0;

So in case a fragment crosses a page boundary, we wrap around to the
beginning of the same page?  That doesn't look right.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 11/21] hwmon: (sht15) Check return value of regulator_enable()
  2013-03-12 22:44 ` [ 11/21] hwmon: (sht15) Check return value of regulator_enable() Greg Kroah-Hartman
@ 2013-03-16  4:15   ` Ben Hutchings
  2013-03-16 13:33     ` Guenter Roeck
  0 siblings, 1 reply; 35+ messages in thread
From: Ben Hutchings @ 2013-03-16  4:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, stable, Mark Brown, Guenter Roeck

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> 3.0-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> commit 3e78080f81481aa8340374d5a37ae033c1cf4272 upstream.
> 
> Not having power is a pretty serious error so check that we are able to
> enable the supply and error out if we can't.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/hwmon/sht15.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> --- a/drivers/hwmon/sht15.c
> +++ b/drivers/hwmon/sht15.c
> @@ -926,7 +926,13 @@ static int __devinit sht15_probe(struct
>  		if (voltage)
>  			data->supply_uV = voltage;
>  
> -		regulator_enable(data->reg);
> +		ret = regulator_enable(data->reg);
> +		if (ret != 0) {
> +			dev_err(&pdev->dev,
> +				"failed to enable regulator: %d\n", ret);
> +			return ret;
> +		}
> +
>  		/*
>  		 * Setup a notifier block to update this if another device
>  		 * causes the voltage to change

Since this has now been released, I think you need this follow-up fix in
3.0.y and 3.4.y:

---
From: Ben Hutchings <ben@decadent.org.uk>
Subject: hwmon: sht15: Fix memory leak if regulator_enable() fails
Date: Sat, 16 Mar 2013 04:11:01 +0000

Commit 3e78080f8148 ('hwmon: (sht15) Check return value of
regulator_enable()') depends on the use of devm_kmalloc() for automatic
resource cleanup in the failure cases, which was introduced in 3.7.  In
older stable branches, explicit cleanup is needed.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
--- a/drivers/hwmon/sht15.c
+++ b/drivers/hwmon/sht15.c
@@ -930,7 +930,7 @@
 		if (ret != 0) {
 			dev_err(&pdev->dev,
 				"failed to enable regulator: %d\n", ret);
-			return ret;
+			goto err_free_data;
 		}
 
 		/*


-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 11/21] hwmon: (sht15) Check return value of regulator_enable()
  2013-03-16  4:15   ` Ben Hutchings
@ 2013-03-16 13:33     ` Guenter Roeck
  0 siblings, 0 replies; 35+ messages in thread
From: Guenter Roeck @ 2013-03-16 13:33 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Greg Kroah-Hartman, linux-kernel, stable, Mark Brown

On Sat, Mar 16, 2013 at 04:15:24AM +0000, Ben Hutchings wrote:
> On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > 
> > commit 3e78080f81481aa8340374d5a37ae033c1cf4272 upstream.
> > 
> > Not having power is a pretty serious error so check that we are able to
> > enable the supply and error out if we can't.
> > 
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> > ---
> >  drivers/hwmon/sht15.c |    8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > --- a/drivers/hwmon/sht15.c
> > +++ b/drivers/hwmon/sht15.c
> > @@ -926,7 +926,13 @@ static int __devinit sht15_probe(struct
> >  		if (voltage)
> >  			data->supply_uV = voltage;
> >  
> > -		regulator_enable(data->reg);
> > +		ret = regulator_enable(data->reg);
> > +		if (ret != 0) {
> > +			dev_err(&pdev->dev,
> > +				"failed to enable regulator: %d\n", ret);
> > +			return ret;
> > +		}
> > +
> >  		/*
> >  		 * Setup a notifier block to update this if another device
> >  		 * causes the voltage to change
> 
> Since this has now been released, I think you need this follow-up fix in
> 3.0.y and 3.4.y:
> 
Excellent catch. I submitted your patch to -stable.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-16  2:10   ` Ben Hutchings
@ 2013-03-18  5:35     ` Asias He
  2013-03-18 21:00       ` Nicholas A. Bellinger
  0 siblings, 1 reply; 35+ messages in thread
From: Asias He @ 2013-03-18  5:35 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Nicholas Bellinger, linux-kernel, stable, Greg Kroah-Hartman

On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Asias He <asias@redhat.com>
> > 
> > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > 
> > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > executed more than one once.
> > 
> > Signed-off-by: Asias He <asias@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > ---
> >  drivers/target/target_core_pscsi.c |    1 -
> >  1 file changed, 1 deletion(-)
> > 
> > --- a/drivers/target/target_core_pscsi.c
> > +++ b/drivers/target/target_core_pscsi.c
> > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> >  				bio = NULL;
> >  			}
> >  
> > -			page++;
> >  			len -= bytes;
> >  			data_len -= bytes;
> >  			off = 0;
> 
> So in case a fragment crosses a page boundary, we wrap around to the
> beginning of the same page?  That doesn't look right.

If the fragment crosses a page boundary, what is the correct page
for it?

Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?

> Ben.
> 
> -- 
> Ben Hutchings
> It is easier to change the specification to fit the program than vice versa.

-- 
Asias

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-18  5:35     ` Asias He
@ 2013-03-18 21:00       ` Nicholas A. Bellinger
  2013-03-18 23:30         ` Ben Hutchings
  0 siblings, 1 reply; 35+ messages in thread
From: Nicholas A. Bellinger @ 2013-03-18 21:00 UTC (permalink / raw)
  To: Asias He
  Cc: Ben Hutchings, linux-kernel, stable, Greg Kroah-Hartman, target-devel

On Mon, 2013-03-18 at 13:35 +0800, Asias He wrote:
> On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> > On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > ------------------
> > > 
> > > From: Asias He <asias@redhat.com>
> > > 
> > > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > > 
> > > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > > executed more than one once.
> > > 
> > > Signed-off-by: Asias He <asias@redhat.com>
> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > ---
> > >  drivers/target/target_core_pscsi.c |    1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > --- a/drivers/target/target_core_pscsi.c
> > > +++ b/drivers/target/target_core_pscsi.c
> > > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> > >  				bio = NULL;
> > >  			}
> > >  
> > > -			page++;
> > >  			len -= bytes;
> > >  			data_len -= bytes;
> > >  			off = 0;
> > 
> > So in case a fragment crosses a page boundary, we wrap around to the
> > beginning of the same page?  That doesn't look right.
> 
> If the fragment crosses a page boundary, what is the correct page
> for it?
> 
> Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?
> 

sg->length + sg->offset can be less than or equal to PAGE_SIZE here.

For everything other than tcm_loop + tcm_vhost using externally
allocated SGLs, we can expect fragments to never cross the page
boundary.

For tcm_loop + tcm_vhost, there are a few special cases with control CDB
paylaods (usually going through scsi-generic) where we can have a non
zero sg->offset, but at least in the cases I've seen this is still not
using SGL elements that exceed PAGE_SIZE.

So, I think this logic is OK for SGLs that cross page boundries, given
that it's done outside of the inner loop where *page is set during each
for_each_sg().  The case where this logic is broken, and that the 'page
++' was addressing is when sg->length > PAGE_SIZE.

--nab


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-18 21:00       ` Nicholas A. Bellinger
@ 2013-03-18 23:30         ` Ben Hutchings
  2013-03-19  0:56           ` Asias He
  0 siblings, 1 reply; 35+ messages in thread
From: Ben Hutchings @ 2013-03-18 23:30 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Asias He, linux-kernel, stable, Greg Kroah-Hartman, target-devel

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

On Mon, 2013-03-18 at 14:00 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2013-03-18 at 13:35 +0800, Asias He wrote:
> > On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> > > On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > > > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > > > 
> > > > ------------------
> > > > 
> > > > From: Asias He <asias@redhat.com>
> > > > 
> > > > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > > > 
> > > > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > > > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > > > executed more than one once.
> > > > 
> > > > Signed-off-by: Asias He <asias@redhat.com>
> > > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > ---
> > > >  drivers/target/target_core_pscsi.c |    1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > --- a/drivers/target/target_core_pscsi.c
> > > > +++ b/drivers/target/target_core_pscsi.c
> > > > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> > > >  				bio = NULL;
> > > >  			}
> > > >  
> > > > -			page++;
> > > >  			len -= bytes;
> > > >  			data_len -= bytes;
> > > >  			off = 0;
> > > 
> > > So in case a fragment crosses a page boundary, we wrap around to the
> > > beginning of the same page?  That doesn't look right.
> > 
> > If the fragment crosses a page boundary, what is the correct page
> > for it?
> > 
> > Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?
> > 
> 
> sg->length + sg->offset can be less than or equal to PAGE_SIZE here.
>
> For everything other than tcm_loop + tcm_vhost using externally
> allocated SGLs, we can expect fragments to never cross the page
> boundary.
> 
> For tcm_loop + tcm_vhost, there are a few special cases with control CDB
> paylaods (usually going through scsi-generic) where we can have a non
> zero sg->offset, but at least in the cases I've seen this is still not
> using SGL elements that exceed PAGE_SIZE.
>
> So, I think this logic is OK for SGLs that cross page boundries, given
> that it's done outside of the inner loop where *page is set during each
> for_each_sg().

The page is set using sg_page() in the outer loop and was then
incremented in the inner loop.

	for_each_sg(sgl, sg, sgl_nents, i) {
		page = sg_page(sg);
		off = sg->offset;
		len = sg->length;

		while (len > 0 && data_len > 0) {
			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
			bytes = min(bytes, data_len);
			...
			/* page++; */
			len -= bytes;
			data_len -= bytes;
			off = 0;
		}
	}

The inner loop is apparently meant to iterate over pages of a segment,
but is now just wrapping around a single page.

> The case where this logic is broken, and that the 'page
> ++' was addressing is when sg->length > PAGE_SIZE.

That is a sufficient but not a necessary condition for the increment.

Ben.

-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-18 23:30         ` Ben Hutchings
@ 2013-03-19  0:56           ` Asias He
  2013-03-19  1:18             ` Ben Hutchings
  0 siblings, 1 reply; 35+ messages in thread
From: Asias He @ 2013-03-19  0:56 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Nicholas A. Bellinger, linux-kernel, stable, Greg Kroah-Hartman,
	target-devel

On Mon, Mar 18, 2013 at 11:30:47PM +0000, Ben Hutchings wrote:
> On Mon, 2013-03-18 at 14:00 -0700, Nicholas A. Bellinger wrote:
> > On Mon, 2013-03-18 at 13:35 +0800, Asias He wrote:
> > > On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> > > > On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > > > > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > > > > 
> > > > > ------------------
> > > > > 
> > > > > From: Asias He <asias@redhat.com>
> > > > > 
> > > > > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > > > > 
> > > > > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > > > > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > > > > executed more than one once.
> > > > > 
> > > > > Signed-off-by: Asias He <asias@redhat.com>
> > > > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > 
> > > > > ---
> > > > >  drivers/target/target_core_pscsi.c |    1 -
> > > > >  1 file changed, 1 deletion(-)
> > > > > 
> > > > > --- a/drivers/target/target_core_pscsi.c
> > > > > +++ b/drivers/target/target_core_pscsi.c
> > > > > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> > > > >  				bio = NULL;
> > > > >  			}
> > > > >  
> > > > > -			page++;
> > > > >  			len -= bytes;
> > > > >  			data_len -= bytes;
> > > > >  			off = 0;
> > > > 
> > > > So in case a fragment crosses a page boundary, we wrap around to the
> > > > beginning of the same page?  That doesn't look right.
> > > 
> > > If the fragment crosses a page boundary, what is the correct page
> > > for it?
> > > 
> > > Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?
> > > 
> > 
> > sg->length + sg->offset can be less than or equal to PAGE_SIZE here.
> >
> > For everything other than tcm_loop + tcm_vhost using externally
> > allocated SGLs, we can expect fragments to never cross the page
> > boundary.
> > 
> > For tcm_loop + tcm_vhost, there are a few special cases with control CDB
> > paylaods (usually going through scsi-generic) where we can have a non
> > zero sg->offset, but at least in the cases I've seen this is still not
> > using SGL elements that exceed PAGE_SIZE.
> >
> > So, I think this logic is OK for SGLs that cross page boundries, given
> > that it's done outside of the inner loop where *page is set during each
> > for_each_sg().
> 
> The page is set using sg_page() in the outer loop and was then
> incremented in the inner loop.
> 
> 	for_each_sg(sgl, sg, sgl_nents, i) {
> 		page = sg_page(sg);
> 		off = sg->offset;
> 		len = sg->length;
> 
> 		while (len > 0 && data_len > 0) {
> 			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
> 			bytes = min(bytes, data_len);
> 			...
> 			/* page++; */
> 			len -= bytes;
> 			data_len -= bytes;
> 			off = 0;
> 		}
> 	}
> 
> The inner loop is apparently meant to iterate over pages of a segment,
> but is now just wrapping around a single page.

Yes, but we never loop more than once in the inner loop.

So, how about
1) fail on  sg->offset + sg->length > PAGE_SIZE (we can not find a
proper page address in this case)
2) remove the inner while loop, run the code was in the loop only once.

> > The case where this logic is broken, and that the 'page
> > ++' was addressing is when sg->length > PAGE_SIZE.
> 
> That is a sufficient but not a necessary condition for the increment.
> 
> Ben.
> 
> -- 
> Ben Hutchings
> Never attribute to conspiracy what can adequately be explained by stupidity.

-- 
Asias

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-19  0:56           ` Asias He
@ 2013-03-19  1:18             ` Ben Hutchings
  2013-03-19  3:18               ` Nicholas A. Bellinger
  0 siblings, 1 reply; 35+ messages in thread
From: Ben Hutchings @ 2013-03-19  1:18 UTC (permalink / raw)
  To: Asias He
  Cc: Nicholas A. Bellinger, linux-kernel, stable, Greg Kroah-Hartman,
	target-devel

[-- Attachment #1: Type: text/plain, Size: 3924 bytes --]

On Tue, 2013-03-19 at 08:56 +0800, Asias He wrote:
> On Mon, Mar 18, 2013 at 11:30:47PM +0000, Ben Hutchings wrote:
> > On Mon, 2013-03-18 at 14:00 -0700, Nicholas A. Bellinger wrote:
> > > On Mon, 2013-03-18 at 13:35 +0800, Asias He wrote:
> > > > On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> > > > > On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > > > > > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > > > > > 
> > > > > > ------------------
> > > > > > 
> > > > > > From: Asias He <asias@redhat.com>
> > > > > > 
> > > > > > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > > > > > 
> > > > > > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > > > > > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > > > > > executed more than one once.
> > > > > > 
> > > > > > Signed-off-by: Asias He <asias@redhat.com>
> > > > > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > 
> > > > > > ---
> > > > > >  drivers/target/target_core_pscsi.c |    1 -
> > > > > >  1 file changed, 1 deletion(-)
> > > > > > 
> > > > > > --- a/drivers/target/target_core_pscsi.c
> > > > > > +++ b/drivers/target/target_core_pscsi.c
> > > > > > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> > > > > >  				bio = NULL;
> > > > > >  			}
> > > > > >  
> > > > > > -			page++;
> > > > > >  			len -= bytes;
> > > > > >  			data_len -= bytes;
> > > > > >  			off = 0;
> > > > > 
> > > > > So in case a fragment crosses a page boundary, we wrap around to the
> > > > > beginning of the same page?  That doesn't look right.
> > > > 
> > > > If the fragment crosses a page boundary, what is the correct page
> > > > for it?
> > > > 
> > > > Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?
> > > > 
> > > 
> > > sg->length + sg->offset can be less than or equal to PAGE_SIZE here.
> > >
> > > For everything other than tcm_loop + tcm_vhost using externally
> > > allocated SGLs, we can expect fragments to never cross the page
> > > boundary.
> > > 
> > > For tcm_loop + tcm_vhost, there are a few special cases with control CDB
> > > paylaods (usually going through scsi-generic) where we can have a non
> > > zero sg->offset, but at least in the cases I've seen this is still not
> > > using SGL elements that exceed PAGE_SIZE.
> > >
> > > So, I think this logic is OK for SGLs that cross page boundries, given
> > > that it's done outside of the inner loop where *page is set during each
> > > for_each_sg().
> > 
> > The page is set using sg_page() in the outer loop and was then
> > incremented in the inner loop.
> > 
> > 	for_each_sg(sgl, sg, sgl_nents, i) {
> > 		page = sg_page(sg);
> > 		off = sg->offset;
> > 		len = sg->length;
> > 
> > 		while (len > 0 && data_len > 0) {
> > 			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
> > 			bytes = min(bytes, data_len);
> > 			...
> > 			/* page++; */
> > 			len -= bytes;
> > 			data_len -= bytes;
> > 			off = 0;
> > 		}
> > 	}
> > 
> > The inner loop is apparently meant to iterate over pages of a segment,
> > but is now just wrapping around a single page.
> 
> Yes, but we never loop more than once in the inner loop.

If you're sure of that, then:

> So, how about
> 1) fail on  sg->offset + sg->length > PAGE_SIZE (we can not find a
> proper page address in this case)
> 2) remove the inner while loop, run the code was in the loop only once.

this is fine, but then I don't understand why you sent a no-op change to
stable in the first place.

Ben.

-- 
Ben Hutchings
When you say `I wrote a program that crashed Windows', people just stare ...
and say `Hey, I got those with the system, *for free*'. - Linus Torvalds

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [ 04/21] target/pscsi: Fix page increment
  2013-03-19  1:18             ` Ben Hutchings
@ 2013-03-19  3:18               ` Nicholas A. Bellinger
  0 siblings, 0 replies; 35+ messages in thread
From: Nicholas A. Bellinger @ 2013-03-19  3:18 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Asias He, linux-kernel, stable, Greg Kroah-Hartman, target-devel

On Tue, 2013-03-19 at 01:18 +0000, Ben Hutchings wrote:
> On Tue, 2013-03-19 at 08:56 +0800, Asias He wrote:
> > On Mon, Mar 18, 2013 at 11:30:47PM +0000, Ben Hutchings wrote:
> > > On Mon, 2013-03-18 at 14:00 -0700, Nicholas A. Bellinger wrote:
> > > > On Mon, 2013-03-18 at 13:35 +0800, Asias He wrote:
> > > > > On Sat, Mar 16, 2013 at 02:10:22AM +0000, Ben Hutchings wrote:
> > > > > > On Tue, 2013-03-12 at 15:44 -0700, Greg Kroah-Hartman wrote:
> > > > > > > 3.0-stable review patch.  If anyone has any objections, please let me know.
> > > > > > > 
> > > > > > > ------------------
> > > > > > > 
> > > > > > > From: Asias He <asias@redhat.com>
> > > > > > > 
> > > > > > > commit 472b72f2db7831d7dbe22ffdff4adee3bd49b05d upstream.
> > > > > > > 
> > > > > > > The page++ is wrong. It makes bio_add_pc_page() pointing to a wrong page
> > > > > > > address if the 'while (len > 0 && data_len > 0) { ... }' loop is
> > > > > > > executed more than one once.
> > > > > > > 
> > > > > > > Signed-off-by: Asias He <asias@redhat.com>
> > > > > > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > > 
> > > > > > > ---
> > > > > > >  drivers/target/target_core_pscsi.c |    1 -
> > > > > > >  1 file changed, 1 deletion(-)
> > > > > > > 
> > > > > > > --- a/drivers/target/target_core_pscsi.c
> > > > > > > +++ b/drivers/target/target_core_pscsi.c
> > > > > > > @@ -1210,7 +1210,6 @@ static int __pscsi_map_task_SG(
> > > > > > >  				bio = NULL;
> > > > > > >  			}
> > > > > > >  
> > > > > > > -			page++;
> > > > > > >  			len -= bytes;
> > > > > > >  			data_len -= bytes;
> > > > > > >  			off = 0;
> > > > > > 
> > > > > > So in case a fragment crosses a page boundary, we wrap around to the
> > > > > > beginning of the same page?  That doesn't look right.
> > > > > 
> > > > > If the fragment crosses a page boundary, what is the correct page
> > > > > for it?
> > > > > 
> > > > > Nicholas, can we assume sg->length + sg->offset should be less than PAGE_SIZE here?
> > > > > 
> > > > 
> > > > sg->length + sg->offset can be less than or equal to PAGE_SIZE here.
> > > >
> > > > For everything other than tcm_loop + tcm_vhost using externally
> > > > allocated SGLs, we can expect fragments to never cross the page
> > > > boundary.
> > > > 
> > > > For tcm_loop + tcm_vhost, there are a few special cases with control CDB
> > > > paylaods (usually going through scsi-generic) where we can have a non
> > > > zero sg->offset, but at least in the cases I've seen this is still not
> > > > using SGL elements that exceed PAGE_SIZE.
> > > >
> > > > So, I think this logic is OK for SGLs that cross page boundries, given
> > > > that it's done outside of the inner loop where *page is set during each
> > > > for_each_sg().
> > > 
> > > The page is set using sg_page() in the outer loop and was then
> > > incremented in the inner loop.
> > > 
> > > 	for_each_sg(sgl, sg, sgl_nents, i) {
> > > 		page = sg_page(sg);
> > > 		off = sg->offset;
> > > 		len = sg->length;
> > > 
> > > 		while (len > 0 && data_len > 0) {
> > > 			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
> > > 			bytes = min(bytes, data_len);
> > > 			...
> > > 			/* page++; */
> > > 			len -= bytes;
> > > 			data_len -= bytes;
> > > 			off = 0;
> > > 		}
> > > 	}
> > > 
> > > The inner loop is apparently meant to iterate over pages of a segment,
> > > but is now just wrapping around a single page.
> > 
> > Yes, but we never loop more than once in the inner loop.
> 
> If you're sure of that, then:
> 
> > So, how about
> > 1) fail on  sg->offset + sg->length > PAGE_SIZE (we can not find a
> > proper page address in this case)
> > 2) remove the inner while loop, run the code was in the loop only once.
> 
> this is fine, but then I don't understand why you sent a no-op change to
> stable in the first place.
> 

It's my fault for mis-understanding this patch, and putting this into
the queue as a stable bug-fix.


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2013-03-19  3:18 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-12 22:44 [ 00/21] 3.0.69-stable review Greg Kroah-Hartman
2013-03-12 22:44 ` [ 01/21] ARM: VFP: fix emulation of second VFP instruction Greg Kroah-Hartman
2013-03-12 22:44 ` [ 02/21] ARM: fix scheduling while atomic warning in alignment handling code Greg Kroah-Hartman
2013-03-12 22:44 ` [ 03/21] SCSI: dc395x: uninitialized variable in device_alloc() Greg Kroah-Hartman
2013-03-12 22:44 ` [ 04/21] target/pscsi: Fix page increment Greg Kroah-Hartman
2013-03-16  2:10   ` Ben Hutchings
2013-03-18  5:35     ` Asias He
2013-03-18 21:00       ` Nicholas A. Bellinger
2013-03-18 23:30         ` Ben Hutchings
2013-03-19  0:56           ` Asias He
2013-03-19  1:18             ` Ben Hutchings
2013-03-19  3:18               ` Nicholas A. Bellinger
2013-03-12 22:44 ` [ 05/21] btrfs: Init io_lock after cloning btrfs device struct Greg Kroah-Hartman
2013-03-12 22:44 ` [ 06/21] cifs: ensure that cifs_get_root() only traverses directories Greg Kroah-Hartman
2013-03-12 22:44 ` [ 07/21] SUNRPC: Dont start the retransmission timer when out of socket space Greg Kroah-Hartman
2013-03-12 22:44 ` [ 08/21] hw_random: make buffer usable in scatterlist Greg Kroah-Hartman
2013-03-12 22:44 ` [ 09/21] ath9k: fix RSSI dummy marker value Greg Kroah-Hartman
2013-03-12 22:44 ` [ 10/21] md: raid0: fix error return from create_stripe_zones Greg Kroah-Hartman
2013-03-12 22:44 ` [ 11/21] hwmon: (sht15) Check return value of regulator_enable() Greg Kroah-Hartman
2013-03-16  4:15   ` Ben Hutchings
2013-03-16 13:33     ` Guenter Roeck
2013-03-12 22:44 ` [ 12/21] drm/radeon: add primary dac adj quirk for R200 board Greg Kroah-Hartman
2013-03-12 22:44 ` [ 13/21] ALSA: ice1712: Initialize card->private_data properly Greg Kroah-Hartman
2013-03-12 22:44 ` [ 14/21] ALSA: vmaster: Fix slave change notification Greg Kroah-Hartman
2013-03-12 22:44 ` [ 15/21] e1000e: fix pci-device enable-counter balance Greg Kroah-Hartman
2013-03-12 22:44 ` [ 16/21] keys: fix race with concurrent install_user_keyrings() Greg Kroah-Hartman
2013-03-12 22:44 ` [ 17/21] vfs: fix pipe counter breakage Greg Kroah-Hartman
2013-03-12 22:44 ` [ 18/21] Fix memory leak in cpufreq stats Greg Kroah-Hartman
2013-03-12 22:44 ` [ 19/21] ftrace: Update the kconfig for DYNAMIC_FTRACE Greg Kroah-Hartman
2013-03-12 22:44 ` [ 20/21] decnet: Fix disappearing sysctl entries Greg Kroah-Hartman
     [not found]   ` <B401117D-23F6-4911-8D72-344EE1A022F6@usgs.gov>
2013-03-12 23:04     ` Eric W. Biederman
     [not found]       ` <3EDA829E-3898-4626-9C17-CCE31E8C0554@usgs.gov>
2013-03-13 20:05         ` Eric W. Biederman
2013-03-12 23:06     ` Greg Kroah-Hartman
2013-03-12 22:44 ` [ 21/21] dmi_scan: fix missing check for _DMI_ signature in smbios_present() Greg Kroah-Hartman
2013-03-13  3:56 ` [ 00/21] 3.0.69-stable review Shuah Khan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.