2008-07-01 15:22:11

by Greg KH

[permalink] [raw]
Subject: [patch 0/9] 2.6.25.10 -stable review

This is the start of the stable review cycle for the 2.6.25.10 release.
There are 9 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.

These patches are sent out with a number of different people on the
Cc: line. If you wish to be a reviewer, please email [email protected]
to add your name to the list. If you want to be off the reviewer list,
also email us.

Responses should be made by July 3, 15: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.25.10-rc1.gz
and the diffstat can be found below.


thanks,

the -stable release team


Makefile | 2
arch/x86/kernel/i387.c | 4 -
arch/x86/kernel/ptrace.c | 45 +++++++------
arch/x86/kernel/smpboot_64.c | 1
drivers/char/drm/i915_drv.c | 1
drivers/infiniband/hw/mthca/mthca_memfree.c | 6 +
drivers/net/hamradio/6pack.c | 2
drivers/net/hamradio/mkiss.c | 8 +-
drivers/net/irda/irtty-sir.c | 4 -
drivers/net/ppp_async.c | 3
drivers/net/ppp_synctty.c | 3
drivers/net/slip.c | 14 +++-
drivers/net/wan/x25_asy.c | 10 ++-
drivers/net/wireless/strip.c | 3
include/asm-x86/msr.h | 2
kernel/futex.c | 93 +++++++++++++++++++++-------
kernel/sched.c | 1
17 files changed, 147 insertions(+), 55 deletions(-)


2008-07-01 15:22:28

by Greg KH

[permalink] [raw]
Subject: [patch 1/9] TTY: fix for tty operations bugs

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

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

From: Alan Cox <[email protected]>

This is fixed with the recent tty operations rewrite in mainline in a
different way, this is a selective backport of the relevant portions to
the -stable tree.

Signed-off-by: Alan Cox <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/net/hamradio/6pack.c | 2 ++
drivers/net/hamradio/mkiss.c | 8 ++++++--
drivers/net/irda/irtty-sir.c | 4 +++-
drivers/net/ppp_async.c | 3 +++
drivers/net/ppp_synctty.c | 3 +++
drivers/net/slip.c | 14 ++++++++++----
drivers/net/wan/x25_asy.c | 10 ++++++++--
drivers/net/wireless/strip.c | 3 ++-
8 files changed, 37 insertions(+), 10 deletions(-)

--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -601,6 +601,8 @@ static int sixpack_open(struct tty_struc

if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (!tty->driver->write)
+ return -EOPNOTSUPP;

dev = alloc_netdev(sizeof(struct sixpack), "sp%d", sp_setup);
if (!dev) {
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -529,6 +529,7 @@ static void ax_encaps(struct net_device
static int ax_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct mkiss *ax = netdev_priv(dev);
+ int cib = 0;

if (!netif_running(dev)) {
printk(KERN_ERR "mkiss: %s: xmit call when iface is down\n", dev->name);
@@ -544,10 +545,11 @@ static int ax_xmit(struct sk_buff *skb,
/* 20 sec timeout not reached */
return 1;
}
+ if (ax->tty->drivers->chars_in_buffer)
+ cib = ax->tty->chars_in_buffer(ax->tty);

printk(KERN_ERR "mkiss: %s: transmit timed out, %s?\n", dev->name,
- (ax->tty->driver->chars_in_buffer(ax->tty) || ax->xleft) ?
- "bad line quality" : "driver error");
+ cib || ax->xleft) ? "bad line quality" : "driver error");

ax->xleft = 0;
clear_bit(TTY_DO_WRITE_WAKEUP, &ax->tty->flags);
@@ -736,6 +738,8 @@ static int mkiss_open(struct tty_struct

if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (!tty->driver->write)
+ return -EOPNOTSUPP;

dev = alloc_netdev(sizeof(struct mkiss), "ax%d", ax_setup);
if (!dev) {
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -64,7 +64,9 @@ static int irtty_chars_in_buffer(struct
IRDA_ASSERT(priv != NULL, return -1;);
IRDA_ASSERT(priv->magic == IRTTY_MAGIC, return -1;);

- return priv->tty->driver->chars_in_buffer(priv->tty);
+ if (priv->tty->drivers->chars_in_buffer)
+ return priv->tty->driver->chars_in_buffer(priv->tty);
+ return 0;
}

/* Wait (sleep) until underlaying hardware finished transmission
--- a/drivers/net/ppp_async.c
+++ b/drivers/net/ppp_async.c
@@ -157,6 +157,9 @@ ppp_asynctty_open(struct tty_struct *tty
{
struct asyncppp *ap;
int err;
+
+ if (!tty->driver->write)
+ return -EOPNOTSUPP;

err = -ENOMEM;
ap = kzalloc(sizeof(*ap), GFP_KERNEL);
--- a/drivers/net/ppp_synctty.c
+++ b/drivers/net/ppp_synctty.c
@@ -207,6 +207,9 @@ ppp_sync_open(struct tty_struct *tty)
struct syncppp *ap;
int err;

+ if (!tty->driver->write)
+ return -EOPNOTSUPP;
+
ap = kzalloc(sizeof(*ap), GFP_KERNEL);
err = -ENOMEM;
if (!ap)
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -460,10 +460,14 @@ static void sl_tx_timeout(struct net_dev
/* 20 sec timeout not reached */
goto out;
}
- printk(KERN_WARNING "%s: transmit timed out, %s?\n",
- dev->name,
- (sl->tty->driver->chars_in_buffer(sl->tty) || sl->xleft) ?
- "bad line quality" : "driver error");
+ {
+ int cib = 0;
+ if (sl->tty->driver->chars_in_buffer)
+ cib = sl->tty->driver->chars_in_buffer(sl->tty);
+ printk(KERN_WARNING "%s: transmit timed out, %s?\n",
+ dev->name, (cib || sl->xleft) ?
+ "bad line quality" : "driver error");
+ }
sl->xleft = 0;
sl->tty->flags &= ~(1 << TTY_DO_WRITE_WAKEUP);
sl_unlock(sl);
@@ -829,6 +833,8 @@ static int slip_open(struct tty_struct *

if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (!tty->driver->write)
+ return -EOPNOTSUPP;

/* RTnetlink lock is misused here to serialize concurrent
opens of slip channels. There are better ways, but it is
--- a/drivers/net/wan/x25_asy.c
+++ b/drivers/net/wan/x25_asy.c
@@ -283,6 +283,10 @@ static void x25_asy_write_wakeup(struct
static void x25_asy_timeout(struct net_device *dev)
{
struct x25_asy *sl = (struct x25_asy*)(dev->priv);
+ int cib = 0;
+
+ if (sl->tty->driver->chars_in_buffer)
+ cib = sl->tty->driver->chars_in_buffer(sl->tty);

spin_lock(&sl->lock);
if (netif_queue_stopped(dev)) {
@@ -290,8 +294,7 @@ static void x25_asy_timeout(struct net_d
* 14 Oct 1994 Dmitry Gorodchanin.
*/
printk(KERN_WARNING "%s: transmit timed out, %s?\n", dev->name,
- (sl->tty->driver->chars_in_buffer(sl->tty) || sl->xleft) ?
- "bad line quality" : "driver error");
+ (cib || sl->xleft) ? "bad line quality" : "driver error");
sl->xleft = 0;
sl->tty->flags &= ~(1 << TTY_DO_WRITE_WAKEUP);
x25_asy_unlock(sl);
@@ -561,6 +564,9 @@ static int x25_asy_open_tty(struct tty_s
return -EEXIST;
}

+ if (!tty->driver->write)
+ return -EOPNOTSUPP;
+
/* OK. Find a free X.25 channel to use. */
if ((sl = x25_asy_alloc()) == NULL) {
return -ENFILE;
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -802,7 +802,8 @@ static void set_baud(struct tty_struct *
struct ktermios old_termios = *(tty->termios);
tty->termios->c_cflag &= ~CBAUD; /* Clear the old baud setting */
tty->termios->c_cflag |= baudcode; /* Set the new baud setting */
- tty->driver->set_termios(tty, &old_termios);
+ if (tty->driver->set_termios)
+ tty->driver->set_termios(tty, &old_termios);
}

/*

--

2008-07-01 15:22:59

by Greg KH

[permalink] [raw]
Subject: [patch 3/9] IB/mthca: Clear ICM pages before handing to FW

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

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

From: Eli Cohen <[email protected]>

commit 87afd448b186c885d67a08b7417cd46253b6a9d6 upstream

Current memfree FW has a bug which in some cases, assumes that ICM
pages passed to it are cleared. This patch uses __GFP_ZERO to
allocate all ICM pages passed to the FW. Once firmware with a fix is
released, we can make the workaround conditional on firmware version.

This fixes the bug reported by Arthur Kepner <[email protected]> here:
http://lists.openfabrics.org/pipermail/general/2008-May/050026.html

[ Rewritten to be a one-liner using __GFP_ZERO instead of vmap()ing
ICM memory and memset()ing it to 0. - Roland ]

Signed-off-by: Eli Cohen <[email protected]>
Signed-off-by: Roland Dreier <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/infiniband/hw/mthca/mthca_memfree.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -109,7 +109,11 @@ static int mthca_alloc_icm_pages(struct
{
struct page *page;

- page = alloc_pages(gfp_mask, order);
+ /*
+ * Use __GFP_ZERO because buggy firmware assumes ICM pages are
+ * cleared, and subtle failures are seen if they aren't.
+ */
+ page = alloc_pages(gfp_mask | __GFP_ZERO, order);
if (!page)
return -ENOMEM;


--

2008-07-01 15:22:43

by Greg KH

[permalink] [raw]
Subject: [patch 2/9] futexes: fix fault handling in futex_lock_pi

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

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

From: Thomas Gleixner <[email protected]>

commit 1b7558e457ed0de61023cfc913d2c342c7c3d9f2 upstream

This patch addresses a very sporadic pi-futex related failure in
highly threaded java apps on large SMP systems.

David Holmes reported that the pi_state consistency check in
lookup_pi_state triggered with his test application. This means that
the kernel internal pi_state and the user space futex variable are out
of sync. First we assumed that this is a user space data corruption,
but deeper investigation revieled that the problem happend because the
pi-futex code is not handling a fault in the futex_lock_pi path when
the user space variable needs to be fixed up.

The fault happens when a fork mapped the anon memory which contains
the futex readonly for COW or the page got swapped out exactly between
the unlock of the futex and the return of either the new futex owner
or the task which was the expected owner but failed to acquire the
kernel internal rtmutex. The current futex_lock_pi() code drops out
with an inconsistent in case it faults and returns -EFAULT to user
space. User space has no way to fixup that state.

When we wrote this code we thought that we could not drop the hash
bucket lock at this point to handle the fault.

After analysing the code again it turned out to be wrong because there
are only two tasks involved which might modify the pi_state and the
user space variable:

- the task which acquired the rtmutex
- the pending owner of the pi_state which did not get the rtmutex

Both tasks drop into the fixup_pi_state() function before returning to
user space. The first task which acquired the hash bucket lock faults
in the fixup of the user space variable, drops the spinlock and calls
futex_handle_fault() to fault in the page. Now the second task could
acquire the hash bucket lock and tries to fixup the user space
variable as well. It either faults as well or it succeeds because the
first task already faulted the page in.

One caveat is to avoid a double fixup. After returning from the fault
handling we reacquire the hash bucket lock and check whether the
pi_state owner has been modified already.

Reported-by: David Holmes <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: David Holmes <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
kernel/futex.c | 93 ++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 20 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1118,21 +1118,64 @@ static void unqueue_me_pi(struct futex_q
* private futexes.
*/
static int fixup_pi_state_owner(u32 __user *uaddr, struct futex_q *q,
- struct task_struct *newowner)
+ struct task_struct *newowner,
+ struct rw_semaphore *fshared)
{
u32 newtid = task_pid_vnr(newowner) | FUTEX_WAITERS;
struct futex_pi_state *pi_state = q->pi_state;
+ struct task_struct *oldowner = pi_state->owner;
u32 uval, curval, newval;
- int ret;
+ int ret, attempt = 0;

/* Owner died? */
+ if (!pi_state->owner)
+ newtid |= FUTEX_OWNER_DIED;
+
+ /*
+ * We are here either because we stole the rtmutex from the
+ * pending owner or we are the pending owner which failed to
+ * get the rtmutex. We have to replace the pending owner TID
+ * in the user space variable. This must be atomic as we have
+ * to preserve the owner died bit here.
+ *
+ * Note: We write the user space value _before_ changing the
+ * pi_state because we can fault here. Imagine swapped out
+ * pages or a fork, which was running right before we acquired
+ * mmap_sem, that marked all the anonymous memory readonly for
+ * cow.
+ *
+ * Modifying pi_state _before_ the user space value would
+ * leave the pi_state in an inconsistent state when we fault
+ * here, because we need to drop the hash bucket lock to
+ * handle the fault. This might be observed in the PID check
+ * in lookup_pi_state.
+ */
+retry:
+ if (get_futex_value_locked(&uval, uaddr))
+ goto handle_fault;
+
+ while (1) {
+ newval = (uval & FUTEX_OWNER_DIED) | newtid;
+
+ curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
+
+ if (curval == -EFAULT)
+ goto handle_fault;
+ if (curval == uval)
+ break;
+ uval = curval;
+ }
+
+ /*
+ * We fixed up user space. Now we need to fix the pi_state
+ * itself.
+ */
if (pi_state->owner != NULL) {
spin_lock_irq(&pi_state->owner->pi_lock);
WARN_ON(list_empty(&pi_state->list));
list_del_init(&pi_state->list);
spin_unlock_irq(&pi_state->owner->pi_lock);
- } else
- newtid |= FUTEX_OWNER_DIED;
+ }

pi_state->owner = newowner;

@@ -1140,26 +1183,35 @@ static int fixup_pi_state_owner(u32 __us
WARN_ON(!list_empty(&pi_state->list));
list_add(&pi_state->list, &newowner->pi_state_list);
spin_unlock_irq(&newowner->pi_lock);
+ return 0;

/*
- * We own it, so we have to replace the pending owner
- * TID. This must be atomic as we have preserve the
- * owner died bit here.
+ * To handle the page fault we need to drop the hash bucket
+ * lock here. That gives the other task (either the pending
+ * owner itself or the task which stole the rtmutex) the
+ * chance to try the fixup of the pi_state. So once we are
+ * back from handling the fault we need to check the pi_state
+ * after reacquiring the hash bucket lock and before trying to
+ * do another fixup. When the fixup has been done already we
+ * simply return.
*/
- ret = get_futex_value_locked(&uval, uaddr);
+handle_fault:
+ spin_unlock(q->lock_ptr);

- while (!ret) {
- newval = (uval & FUTEX_OWNER_DIED) | newtid;
+ ret = futex_handle_fault((unsigned long)uaddr, fshared, attempt++);

- curval = cmpxchg_futex_value_locked(uaddr, uval, newval);
+ spin_lock(q->lock_ptr);

- if (curval == -EFAULT)
- ret = -EFAULT;
- if (curval == uval)
- break;
- uval = curval;
- }
- return ret;
+ /*
+ * Check if someone else fixed it for us:
+ */
+ if (pi_state->owner != oldowner)
+ return 0;
+
+ if (ret)
+ return ret;
+
+ goto retry;
}

/*
@@ -1524,7 +1576,7 @@ static int futex_lock_pi(u32 __user *uad
* that case:
*/
if (q.pi_state->owner != curr)
- ret = fixup_pi_state_owner(uaddr, &q, curr);
+ ret = fixup_pi_state_owner(uaddr, &q, curr, fshared);
} else {
/*
* Catch the rare case, where the lock was released
@@ -1556,7 +1608,8 @@ static int futex_lock_pi(u32 __user *uad
int res;

owner = rt_mutex_owner(&q.pi_state->pi_mutex);
- res = fixup_pi_state_owner(uaddr, &q, owner);
+ res = fixup_pi_state_owner(uaddr, &q, owner,
+ fshared);

/* propagate -EFAULT, if the fixup failed */
if (res)

--

2008-07-01 15:23:24

by Greg KH

[permalink] [raw]
Subject: [patch 4/9] DRM: enable bus mastering on i915 at resume time

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

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

From: Jie Luo <[email protected]>

commit ea7b44c8e6baa1a4507f05ba2c0009ac21c3fe0b upstream

On 9xx chips, bus mastering needs to be enabled at resume time for much of the
chip to function. With this patch, vblank interrupts will work as expected
on resume, along with other chip functions. Fixes kernel bugzilla #10844.

Signed-off-by: Jie Luo <[email protected]>
Signed-off-by: Jesse Barnes <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/char/drm/i915_drv.c | 1 +
1 file changed, 1 insertion(+)

--- a/drivers/char/drm/i915_drv.c
+++ b/drivers/char/drm/i915_drv.c
@@ -385,6 +385,7 @@ static int i915_resume(struct drm_device
pci_restore_state(dev->pdev);
if (pci_enable_device(dev->pdev))
return -1;
+ pci_set_master(dev->pdev);

pci_write_config_byte(dev->pdev, LBB, dev_priv->saveLBB);


--

2008-07-01 15:23:54

by Greg KH

[permalink] [raw]
Subject: [patch 6/9] sched: fix cpu hotplug

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

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

From: Dmitry Adamushko <[email protected]>

Commit 79c537998d143b127c8c662a403c3356cb885f1c upstream

the CPU hotplug problems (crashes under high-volume unplug+replug
tests) seem to be related to migrate_dead_tasks().

Firstly I added traces to see all tasks being migrated with
migrate_live_tasks() and migrate_dead_tasks(). On my setup the problem
pops up (the one with "se == NULL" in the loop of
pick_next_task_fair()) shortly after the traces indicate that some has
been migrated with migrate_dead_tasks()). btw., I can reproduce it
much faster now with just a plain cpu down/up loop.

[disclaimer] Well, unless I'm really missing something important in
this late hour [/desclaimer] pick_next_task() is not something
appropriate for migrate_dead_tasks() :-)

the following change seems to eliminate the problem on my setup
(although, I kept it running only for a few minutes to get a few
messages indicating migrate_dead_tasks() does move tasks and the
system is still ok)

Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
kernel/sched.c | 1 +
1 file changed, 1 insertion(+)

--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -5728,6 +5728,7 @@ static void migrate_dead_tasks(unsigned
next = pick_next_task(rq, rq->curr);
if (!next)
break;
+ next->sched_class->put_prev_task(rq, next);
migrate_dead(dead_cpu, next);

}

--

2008-07-01 15:23:39

by Greg KH

[permalink] [raw]
Subject: [patch 5/9] x86_64 ptrace: fix sys32_ptrace task_struct leak

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

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

From: Roland McGrath <[email protected]>

Commit 5a4646a4efed8c835f76c3b88f3155f6ab5b8d9b introduced a leak of
task_struct refs into sys32_ptrace. This bug has already gone away in
for 2.6.26 in commit 562b80bafffaf42a6d916b0a2ee3d684220a1c10.

Signed-off-by: Roland McGrath <[email protected]>
Acked-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
arch/x86/kernel/ptrace.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)

--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1309,42 +1309,49 @@ asmlinkage long sys32_ptrace(long reques
break;

case PTRACE_GETREGS: /* Get all gp regs from the child. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_GENERAL,
- 0, sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_GENERAL,
+ 0, sizeof(struct user_regs_struct32),
+ datap);
+ break;

case PTRACE_SETREGS: /* Set all gp regs in the child. */
- return copy_regset_from_user(child, &user_x86_32_view,
- REGSET_GENERAL, 0,
- sizeof(struct user_regs_struct32),
- datap);
+ ret = copy_regset_from_user(child, &user_x86_32_view,
+ REGSET_GENERAL, 0,
+ sizeof(struct user_regs_struct32),
+ datap);
+ break;

case PTRACE_GETFPREGS: /* Get the child FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_FP, 0,
- sizeof(struct user_i387_ia32_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_FP, 0,
+ sizeof(struct user_i387_ia32_struct),
+ datap);
+ break;

case PTRACE_SETFPREGS: /* Set the child FPU state. */
- return copy_regset_from_user(
+ ret = copy_regset_from_user(
child, &user_x86_32_view, REGSET_FP,
0, sizeof(struct user_i387_ia32_struct), datap);
+ break;

case PTRACE_GETFPXREGS: /* Get the child extended FPU state. */
- return copy_regset_to_user(child, &user_x86_32_view,
- REGSET_XFP, 0,
- sizeof(struct user32_fxsr_struct),
- datap);
+ ret = copy_regset_to_user(child, &user_x86_32_view,
+ REGSET_XFP, 0,
+ sizeof(struct user32_fxsr_struct),
+ datap);
+ break;

case PTRACE_SETFPXREGS: /* Set the child extended FPU state. */
- return copy_regset_from_user(child, &user_x86_32_view,
+ ret = copy_regset_from_user(child, &user_x86_32_view,
REGSET_XFP, 0,
sizeof(struct user32_fxsr_struct),
datap);
+ break;

default:
- return compat_ptrace_request(child, request, addr, data);
+ ret = compat_ptrace_request(child, request, addr, data);
+ break;
}

out:

--

2008-07-01 15:24:25

by Greg KH

[permalink] [raw]
Subject: [patch 7/9] ptrace GET/SET FPXREGS broken

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

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

From: TAKADA Yoshihito <[email protected]>

Commit 11dbc963a8f6128595d0f6ecf138dc369e144997 upstream

ptrace GET/SET FPXREGS broken

When I update kernel 2.6.25 from 2.6.24, gdb does not work.
On 2.6.25, ptrace(PTRACE_GETFPXREGS, ...) returns ENODEV.

But 2.6.24 kernel's ptrace() returns EIO.
It is issue of compatibility.

I attached test program as pt.c and patch for fix it.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/types.h>

struct user_fxsr_struct {
unsigned short cwd;
unsigned short swd;
unsigned short twd;
unsigned short fop;
long fip;
long fcs;
long foo;
long fos;
long mxcsr;
long reserved;
long st_space[32]; /* 8*16 bytes for each FP-reg = 128 bytes */
long xmm_space[32]; /* 8*16 bytes for each XMM-reg = 128 bytes */
long padding[56];
};

int main(void)
{
pid_t pid;

pid = fork();

switch(pid){
case -1:/* error */
break;
case 0:/* child */
child();
break;
default:
parent(pid);
break;
}
return 0;
}

int child(void)
{
ptrace(PTRACE_TRACEME);
kill(getpid(), SIGSTOP);
sleep(10);
return 0;
}
int parent(pid_t pid)
{
int ret;
struct user_fxsr_struct fpxregs;

ret = ptrace(PTRACE_GETFPXREGS, pid, 0, &fpxregs);
if(ret < 0){
printf("%d: %s.\n", errno, strerror(errno));
}
kill(pid, SIGCONT);
wait(pid);
return 0;
}

/* in the kerel, at kernel/i387.c get_fpxregs() */

Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
arch/x86/kernel/i387.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -130,7 +130,7 @@ int xfpregs_get(struct task_struct *targ
void *kbuf, void __user *ubuf)
{
if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;

init_fpu(target);

@@ -145,7 +145,7 @@ int xfpregs_set(struct task_struct *targ
int ret;

if (!cpu_has_fxsr)
- return -ENODEV;
+ return -EIO;

init_fpu(target);
set_stopped_child_used_math(target);

--

2008-07-01 15:24:43

by Greg KH

[permalink] [raw]
Subject: [patch 8/9] x86: fix cpu hotplug crash

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

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

From: Yanmin Zhang <[email protected]>

Commit fcb43042ef55d2f46b0efa5d7746967cef38f056 upstream

x86: fix cpu hotplug crash

Vegard Nossum reported crashes during cpu hotplug tests:

http://marc.info/?l=linux-kernel&m=121413950227884&w=4

In function _cpu_up, the panic happens when calling
__raw_notifier_call_chain at the second time. Kernel doesn't panic when
calling it at the first time. If just say because of nr_cpu_ids, that's
not right.

By checking the source code, I found that function do_boot_cpu is the culprit.
Consider below call chain:
_cpu_up=>__cpu_up=>smp_ops.cpu_up=>native_cpu_up=>do_boot_cpu.

So do_boot_cpu is called in the end. In do_boot_cpu, if
boot_error==true, cpu_clear(cpu, cpu_possible_map) is executed. So later
on, when _cpu_up calls __raw_notifier_call_chain at the second time to
report CPU_UP_CANCELED, because this cpu is already cleared from
cpu_possible_map, get_cpu_sysdev returns NULL.

Many resources are related to cpu_possible_map, so it's better not to
change it.

Below patch against 2.6.26-rc7 fixes it by removing the bit clearing in
cpu_possible_map.

Signed-off-by: Zhang Yanmin <[email protected]>
Tested-by: Vegard Nossum <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>


---
arch/x86/kernel/smpboot_64.c | 1 -
1 file changed, 1 deletion(-)

--- a/arch/x86/kernel/smpboot_64.c
+++ b/arch/x86/kernel/smpboot_64.c
@@ -704,7 +704,6 @@ do_rest:
clear_bit(cpu, (unsigned long *)&cpu_initialized); /* was set by cpu_init() */
clear_node_cpumask(cpu); /* was set by numa_add_cpu */
cpu_clear(cpu, cpu_present_map);
- cpu_clear(cpu, cpu_possible_map);
per_cpu(x86_cpu_to_apicid, cpu) = BAD_APICID;
return -EIO;
}

--

2008-07-01 15:24:58

by Greg KH

[permalink] [raw]
Subject: [patch 9/9] x86: shift bits the right way in native_read_tscp

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

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

From: Max Asbock <[email protected]>

Commit 41aefdcc98fdba47459eab67630293d67e855fc3 upstream

x86: shift bits the right way in native_read_tscp

native_read_tscp shifts the bits in the high order value in the
wrong direction, the attached patch fixes that.

Signed-off-by: Max Asbock <[email protected]>
Acked-by: Glauber Costa <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
include/asm-x86/msr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/asm-x86/msr.h
+++ b/include/asm-x86/msr.h
@@ -18,7 +18,7 @@ static inline unsigned long long native_
unsigned long low, high;
asm volatile (".byte 0x0f,0x01,0xf9"
: "=a" (low), "=d" (high), "=c" (*aux));
- return low | ((u64)high >> 32);
+ return low | ((u64)high << 32);
}

/*

--

2008-07-01 16:03:20

by Greg KH

[permalink] [raw]
Subject: Re: [patch 1/9] TTY: fix for tty operations bugs

On Tue, Jul 01, 2008 at 08:18:59AM -0700, Greg KH wrote:
> 2.6.25-stable review patch. If anyone has any objections, please let us know.
>
> ------------------
>
> From: Alan Cox <[email protected]>
>
> This is fixed with the recent tty operations rewrite in mainline in a
> different way, this is a selective backport of the relevant portions to
> the -stable tree.
>
> Signed-off-by: Alan Cox <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> drivers/net/hamradio/6pack.c | 2 ++
> drivers/net/hamradio/mkiss.c | 8 ++++++--
> drivers/net/irda/irtty-sir.c | 4 +++-

Hm, there are a few build errors with this patch in these two files,
I'll go respin it and do a new -rc release...

Sorry for missing this before.

thanks,

greg k-h

2008-07-01 16:45:07

by Greg KH

[permalink] [raw]
Subject: Re: [patch 0/9] 2.6.25.10 -stable review

On Tue, Jul 01, 2008 at 08:18:35AM -0700, Greg KH wrote:
> This is the start of the stable review cycle for the 2.6.25.10 release.
> There are 9 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.
>
> These patches are sent out with a number of different people on the
> Cc: line. If you wish to be a reviewer, please email [email protected]
> to add your name to the list. If you want to be off the reviewer list,
> also email us.
>
> Responses should be made by July 3, 15: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.25.10-rc1.gz
> and the diffstat can be found below.

I have released 2.6.25.10-rc2 at:
kernel.org/pub/linux/kernel/v2.6/stable-review/patch-2.6.25.10-rc2.gz
due to a build error in the -rc1 patch.

Sorry about that.

thanks,

greg k-h

2008-07-02 09:58:03

by S.Çağlar Onur

[permalink] [raw]
Subject: Re: [patch 1/9] TTY: fix for tty operations bugs

Hi Greg;

01 Tem 2008 Sal tarihinde, Greg KH şunları yazmıştı:
> On Tue, Jul 01, 2008 at 08:18:59AM -0700, Greg KH wrote:
> > 2.6.25-stable review patch. If anyone has any objections, please let us know.
> >
> > ------------------
> >
> > From: Alan Cox <[email protected]>
> >
> > This is fixed with the recent tty operations rewrite in mainline in a
> > different way, this is a selective backport of the relevant portions to
> > the -stable tree.
> >
> > Signed-off-by: Alan Cox <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
> > ---
> > drivers/net/hamradio/6pack.c | 2 ++
> > drivers/net/hamradio/mkiss.c | 8 ++++++--
> > drivers/net/irda/irtty-sir.c | 4 +++-
>
> Hm, there are a few build errors with this patch in these two files,
> I'll go respin it and do a new -rc release...
>
> Sorry for missing this before.

I still have following build error

[...]
drivers/net/hamradio/mkiss.c: In function 'ax_xmit':
drivers/net/hamradio/mkiss.c:548: error: 'struct tty_struct' has no member named 'drivers'
drivers/net/hamradio/mkiss.c:549: error: 'struct tty_struct' has no member named 'chars_in_buffer'
drivers/net/hamradio/mkiss.c:552: warning: format '%s' expects type 'char *', but argument 3 has type 'int'
drivers/net/hamradio/mkiss.c:552: error: expected ';' before ')' token
drivers/net/hamradio/mkiss.c:552: error: expected statement before ')' token

*** 4 errors, 1 warnings
[...]

after your

commit bdd6248729d1f4a75e4623bce4f9c7737753b712
Author: Greg Kroah-Hartman <[email protected]>
Date: Tue Jul 1 09:44:04 2008 -0700

fix build error in tty patch

commit to stable-queue

Cheers
--
S.Çağlar Onur <[email protected]>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

2008-07-02 10:05:16

by Alan

[permalink] [raw]
Subject: Re: [patch 1/9] TTY: fix for tty operations bugs


> drivers/net/hamradio/mkiss.c: In function 'ax_xmit':
> drivers/net/hamradio/mkiss.c:548: error: 'struct tty_struct' has no member named 'drivers'
> drivers/net/hamradio/mkiss.c:549: error: 'struct tty_struct' has no member named 'chars_in_buffer'
> drivers/net/hamradio/mkiss.c:552: warning: format '%s' expects type 'char *', but argument 3 has type 'int'
> drivers/net/hamradio/mkiss.c:552: error: expected ';' before ')' token
> drivers/net/hamradio/mkiss.c:552: error: expected statement before ')' token

Eugene Teo posted the patch to fix that.

Alan

2008-07-02 14:46:44

by Greg KH

[permalink] [raw]
Subject: Re: [patch 1/9] TTY: fix for tty operations bugs

On Wed, Jul 02, 2008 at 12:57:44PM +0300, S.Çağlar Onur wrote:
> Hi Greg;
>
> 01 Tem 2008 Sal tarihinde, Greg KH şunları yazmıştı:
> > On Tue, Jul 01, 2008 at 08:18:59AM -0700, Greg KH wrote:
> > > 2.6.25-stable review patch. If anyone has any objections, please let us know.
> > >
> > > ------------------
> > >
> > > From: Alan Cox <[email protected]>
> > >
> > > This is fixed with the recent tty operations rewrite in mainline in a
> > > different way, this is a selective backport of the relevant portions to
> > > the -stable tree.
> > >
> > > Signed-off-by: Alan Cox <[email protected]>
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > >
> > > ---
> > > drivers/net/hamradio/6pack.c | 2 ++
> > > drivers/net/hamradio/mkiss.c | 8 ++++++--
> > > drivers/net/irda/irtty-sir.c | 4 +++-
> >
> > Hm, there are a few build errors with this patch in these two files,
> > I'll go respin it and do a new -rc release...
> >
> > Sorry for missing this before.
>
> I still have following build error
>
> [...]
> drivers/net/hamradio/mkiss.c: In function 'ax_xmit':
> drivers/net/hamradio/mkiss.c:548: error: 'struct tty_struct' has no member named 'drivers'
> drivers/net/hamradio/mkiss.c:549: error: 'struct tty_struct' has no member named 'chars_in_buffer'
> drivers/net/hamradio/mkiss.c:552: warning: format '%s' expects type 'char *', but argument 3 has type 'int'
> drivers/net/hamradio/mkiss.c:552: error: expected ';' before ')' token
> drivers/net/hamradio/mkiss.c:552: error: expected statement before ')' token
>
> *** 4 errors, 1 warnings
> [...]
>
> after your
>
> commit bdd6248729d1f4a75e4623bce4f9c7737753b712
> Author: Greg Kroah-Hartman <[email protected]>
> Date: Tue Jul 1 09:44:04 2008 -0700
>
> fix build error in tty patch
>
> commit to stable-queue

Are you sure you redid your patch set, I see fixes for the error
messages above in that patch I did. I also don't get the build problems
myself anymore.

confused,

greg k-h

2008-07-02 15:09:23

by S.Çağlar Onur

[permalink] [raw]
Subject: Re: [patch 1/9] TTY: fix for tty operations bugs

Hi;

02 Tem 2008 Çar tarihinde, Greg KH şunları yazmıştı:
> Are you sure you redid your patch set, I see fixes for the error
> messages above in that patch I did. I also don't get the build problems
> myself anymore.

I start to hate myself :(, as you said refreshing whole patchset with quilt solved the compiliation problem, i thought i did refresh but i didn't, and compiled kernel works fine. I really have to drink some coffee.

> confused,

Sorry for your time...

> greg k-h

Cheers
--
S.Çağlar Onur <[email protected]>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

2008-07-16 04:52:37

by Greg KH

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Wed, Jul 16, 2008 at 01:01:24AM -0300, Rodrigo Rubira Branco wrote:
> First of all sorry for copy many people who maybe are not in the initial
> discussion, but since I've not been copied I have no idea who are and who
> are not in that thread ;)
>
> The point that many people are trying to make is that Linux has a policy
> defined in a document (Documentation/SecurityBugs) but are not following it.

{sigh}

Are you sure? What specific sentance(s) are you saying that we are not
currently following. And if not, can you propose a patch to the file to
properly reflect how you seem to think we currently are working?

getting very annoyed at people saying I am somehow doing the job I do
for free, on my own time, incorrectly,

greg k-h

Subject: Re: [stable] Linux 2.6.25.10 (resume)

First of all sorry for copy many people who maybe are not in the initial
discussion, but since I've not been copied I have no idea who are and who
are not in that thread ;)

The point that many people are trying to make is that Linux has a policy
defined in a document (Documentation/SecurityBugs) but are not following it.

Don't really matter to us what the policy is, but it's really important to
follow it (many people, who are security professionals need that, and many
others, who are NOT security professionals also).

We all know (both sides) that it's impossible to know everything related to
every bug and it's security impact. But there is a lot of different
situations well-known as a security problems (because the bug class is well
know, because someone reported it with details to the devels, etc). Hide it
is an option, disclouse it is another. Have a policy is what matters. Say
something and do another thing is always bad to everybody involved.


P.S: I'm talking by myself, not for the company that I work for.


Rodrigo Rubira Branco (BSDaemon).

Subject: Re: [stable] Linux 2.6.25.10 (resume)

--- SecurityBugs.orig 2008-07-16 23:46:09.000000000 -0300
+++ SecurityBugs 2008-07-17 14:58:32.000000000 -0300
@@ -1,7 +1,7 @@
-Linux kernel developers take security very seriously. As such, we'd
-like to know when a security bug is found so that it can be fixed and
-disclosed as quickly as possible. Please report security bugs to the
-Linux kernel security team.
+Linux kernel developers take security very seriously, in exactly the
+same way we do with any other bugs. As such, we'd like to know when
+a security bug is found so that it can be fixed as soon as possible.
+Please report security bugs to the Linux kernel security team.

1) Contact

@@ -14,23 +14,24 @@
As it is with any bug, the more information provided the easier it
will be to diagnose and fix. Please review the procedure outlined in
REPORTING-BUGS if you are unclear about what information is helpful.
-Any exploit code is very helpful and will not be released without
-consent from the reporter unless it has already been made public.
+Any exploit code is very helpful and will not be released.

2) Disclosure

The goal of the Linux kernel security team is to work with the
bug submitter to bug resolution as well as disclosure. We prefer
-to fully disclose the bug as soon as possible. It is reasonable to
+to not disclose the bug, since we believe any kind of bug deserves the
+same attention and will be quickly patched. It is reasonable to
delay disclosure when the bug or the fix is not yet fully understood,
the solution is not well-tested or for vendor coordination. However, we
expect these delays to be short, measurable in days, not weeks or months.
A disclosure date is negotiated by the security team working with the
-bug submitter as well as vendors. However, the kernel security team
-holds the final say when setting a disclosure date. The timeframe for
-disclosure is from immediate (esp. if it's already publically known)
-to a few weeks. As a basic default policy, we expect report date to
-disclosure date to be on the order of 7 days.
+bug submitter as well as vendors if the submitter wants to disclose.
+However, the kernel security team holds the final say when setting a
+disclosure date. The timeframe for disclousure is from immediate (esp. if
+it's already publically known) to a few weeks. As a basic default policy,
+we expect report date to disclosure (if the submitter requires disclosure)
+to be on the order of 7 days.

3) Non-disclosure agreements



Attachments:
SecurityBugs.patch (2.44 kB)

2008-07-18 15:23:46

by Willy Tarreau

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Fri, Jul 18, 2008 at 11:07:45AM -0300, Rodrigo Rubira Branco (BSDaemon) wrote:
> >getting very annoyed at people saying I am somehow doing the job I do
> >for free, on my own time, incorrectly,
> >
> >greg k-h
> >
> For free? Hum, let's forget that you work as a linux developer... we are
> also trying to contribute in some way for free...

Rodrigo, this type of personal comment is unjustified and very inappropriate
IMHO. It's not fair at all to judge people's personal involvement based on
whom they work for. If you'd work at nights and week-ends after your job to
backport bugfixes, you'd know what I'm talking about. BTW, one day you may
notice that the most personally involved ones *finally* get a job at a Linux
vendor's, and not the reverse.

So please take care of not dismissing what people do on their own time, and
try be kind enough to bring problems to them directly if any.

Thanks,
Willy

Subject: Re: [stable] Linux 2.6.25.10 (resume)

Willy Tarreau escreveu:
> On Fri, Jul 18, 2008 at 11:07:45AM -0300, Rodrigo Rubira Branco (BSDaemon) wrote:
>
>>> getting very annoyed at people saying I am somehow doing the job I do
>>> for free, on my own time, incorrectly,
>>>
>>> greg k-h
>>>
>>>
>> For free? Hum, let's forget that you work as a linux developer... we are
>> also trying to contribute in some way for free...
>>
>
> Rodrigo, this type of personal comment is unjustified and very inappropriate
> IMHO. It's not fair at all to judge people's personal involvement based on
> whom they work for. If you'd work at nights and week-ends after your job to
> backport bugfixes, you'd know what I'm talking about. BTW, one day you may
> notice that the most personally involved ones *finally* get a job at a Linux
> vendor's, and not the reverse.
>
> So please take care of not dismissing what people do on their own time, and
> try be kind enough to bring problems to them directly if any.
>
> Thanks,
> Willy
>
>

Hey Willy,

Sorry, but my comment was not unjustified, since he started saying he is
doing a free-job and we are saying is doing it 'wrong'.... That's not
the case. We all are doing a free job, contributing in some way, not
just you, him or me... Everybody involved in this discussion.

But he, more than others, are also paid to do that job... Also, as a
developer of any project so important as linux, doing that for free or
not, you have a responsibility.
You can choose to take it or not (I think that's why there is not many
people with balls to accept it, and I congrats Greg for that).

Just to finish my statement, let's stop this 'forks' on the main thread,
please. I'm not here to judge anybody. Since it's an open project, we
are trying to give our contribution as security researchers and as
people who understand the security marketing.


P.S.: Blahblahblah, disclaimer that this is my opinion and not the
opinion of the company that I work for.


Regards,


Rodrigo (BSDaemon).

2008-07-19 04:47:26

by David Lang

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Fri, 18 Jul 2008, Rodrigo Rubira Branco (BSDaemon) wrote:

> Willy Tarreau escreveu:
>> On Fri, Jul 18, 2008 at 11:07:45AM -0300, Rodrigo Rubira Branco (BSDaemon)
>> wrote:
>>
>>>> getting very annoyed at people saying I am somehow doing the job I do
>>>> for free, on my own time, incorrectly,
>>>>
>>>> greg k-h
>>>>
>>> For free? Hum, let's forget that you work as a linux developer... we are
>>> also trying to contribute in some way for free...
>>>
>>
>> Rodrigo, this type of personal comment is unjustified and very
>> inappropriate
>> IMHO. It's not fair at all to judge people's personal involvement based on
>> whom they work for. If you'd work at nights and week-ends after your job to
>> backport bugfixes, you'd know what I'm talking about. BTW, one day you may
>> notice that the most personally involved ones *finally* get a job at a
>> Linux
>> vendor's, and not the reverse.
>>
>> So please take care of not dismissing what people do on their own time, and
>> try be kind enough to bring problems to them directly if any.
>>
>> Thanks,
>> Willy
>>
>>
>
> Hey Willy,
>
> Sorry, but my comment was not unjustified, since he started saying he is
> doing a free-job and we are saying is doing it 'wrong'.... That's not the
> case. We all are doing a free job, contributing in some way, not just you,
> him or me... Everybody involved in this discussion.
> But he, more than others, are also paid to do that job...

he's paied to work on what his company chooses to have him do for Linux.
Greg is saying that he is doing all the -stable work on his own time,
after finishing the work on Linux that his employer chooses to pay him
for.

> Also, as a
> developer of any project so important as linux, doing that for free or not,
> you have a responsibility.
> You can choose to take it or not (I think that's why there is not many
> people with balls to accept it, and I congrats Greg for that).

if you would define the job up front and ask for someone to take it on you
could then argue that they aren't doing it right. but when someone
volunteers to do something and creates the job, claiming that they re
doing it 'wrong' after the fact is insulting. the job is what he defined
it to be when he created it. if you want to define the job differently you
are welcome to do just that and start doing the different job (or hire
someone to do the job). if you are right then Greg's work will be
irrelavent and he can stop wasting his time. but unless you are willing to
step up you should not take the tone that you have taken. you can ask for
changes, explain why you think they are better, try and convince others,
but you cannot say that he must or that he has a responsibility to do it
your way.

David Lang

> Just to finish my statement, let's stop this 'forks' on the main thread,
> please. I'm not here to judge anybody. Since it's an open project, we are
> trying to give our contribution as security researchers and as people who
> understand the security marketing.
>
>
> P.S.: Blahblahblah, disclaimer that this is my opinion and not the opinion
> of the company that I work for.
>
>
> Regards,
>
>
> Rodrigo (BSDaemon).
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-07-19 10:22:24

by Alan Cox

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

> @@ -1,7 +1,7 @@
> -Linux kernel developers take security very seriously. As such, we'd
> -like to know when a security bug is found so that it can be fixed and
> -disclosed as quickly as possible. Please report security bugs to the
> -Linux kernel security team.
> +Linux kernel developers take security very seriously, in exactly the
> +same way we do with any other bugs. As such, we'd like to know when
> +a security bug is found so that it can be fixed as soon as possible.
> +Please report security bugs to the Linux kernel security team.

NAK this. If the fix is not clear and the bug not too serious it is better
to disclose it than fail to fix it. The security team does not usually fix the
bugs, the experts in the various bits of code do.

> -Any exploit code is very helpful and will not be released without
> -consent from the reporter unless it has already been made public.
> +Any exploit code is very helpful and will not be released.

NAK this too. If someone releases an exploit publically or it leaks we
want to be able to freely share it too. Your proposal would mean any but
those dumb enough to agree to this could share it. That is why the unless made
public is part of every generic NDA document on the planet.


The rest needs Linus to return from holiday for discussion and that'll
be a week or two. In the meantime you might want to define "disclose" as
I don't think we all agree on what it means as you've not defined who is and
isn't the linux security team and/or its helpers.

2008-07-19 22:19:23

by Greg KH

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Fri, Jul 18, 2008 at 11:07:45AM -0300, Rodrigo Rubira Branco (BSDaemon) wrote:
> --- SecurityBugs.orig 2008-07-16 23:46:09.000000000 -0300
> +++ SecurityBugs 2008-07-17 14:58:32.000000000 -0300
> @@ -1,7 +1,7 @@
> -Linux kernel developers take security very seriously. As such, we'd
> -like to know when a security bug is found so that it can be fixed and
> -disclosed as quickly as possible. Please report security bugs to the
> -Linux kernel security team.
> +Linux kernel developers take security very seriously, in exactly the
> +same way we do with any other bugs. As such, we'd like to know when
> +a security bug is found so that it can be fixed as soon as possible.
> +Please report security bugs to the Linux kernel security team.

I guess what is getting everyone's panties all in a bind is the term
"disclosed", right? Why not just drop this word from the sentence
instead of rewording it so much?

> @@ -14,23 +14,24 @@
> As it is with any bug, the more information provided the easier it
> will be to diagnose and fix. Please review the procedure outlined in
> REPORTING-BUGS if you are unclear about what information is helpful.
> -Any exploit code is very helpful and will not be released without
> -consent from the reporter unless it has already been made public.
> +Any exploit code is very helpful and will not be released.

I don't see why this needs to be changed, sometimes we do release
exploit code to third parties that ask us nicely and the reporter allows
us to.

> 2) Disclosure
>
> The goal of the Linux kernel security team is to work with the
> bug submitter to bug resolution as well as disclosure. We prefer
> -to fully disclose the bug as soon as possible.

Ah, again, it's the "fully disclose" that is causing panties to ride
high. And again, we are disclosing the bug with the real fix and the
code in question. We just seem to differ on what people consider
"fully" it seems. I think the people liking that term these days
consider that you must release exploit and other detailed information.

I disagree with this and feel that our current policy of fixing bugs and
releasing full code is pretty much the same thing as we are doing today,
although I can understand the confusion. How about this rewording of
the sentance instead:

We prefer to fix and provide an update for the bug as soon as
possible.

So a simple 1 line change should be enough to stem this kind of argument
in the future, right?

thanks,

greg k-h

2008-07-20 17:29:14

by Al Viro

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Sat, Jul 19, 2008 at 03:13:43PM -0700, Greg KH wrote:

> I disagree with this and feel that our current policy of fixing bugs and
> releasing full code is pretty much the same thing as we are doing today,
> although I can understand the confusion. How about this rewording of
> the sentance instead:
>
> We prefer to fix and provide an update for the bug as soon as
> possible.
>
> So a simple 1 line change should be enough to stem this kind of argument
> in the future, right?

Not quite... OK, here's a story that might serve as a model
of all that crap - it certainly runs afoul of a bunch of arguments on
all sides of that.

We all know that POSIX locks suck by design, in particular where it
deals with close(2) semantics. "$FOO is associated with process P having
a descriptor refering to opened file F, $FOO disappears when any of such
descriptors get removed" is bloody inconvenient in a lot of respects. It
also turns out to invite very similar kind of wrong assumptions in all
implementation that have to deal with descriptor tables being possibly
shared. So far the victims include:
* FreeBSD POSIX locks; used to be vulnerable, fixed.
* OpenBSD POSIX locks; vulnerable.
* Linux POSIX locks and dnotify entries; used to be vulnerable, fixed.
Plan9 happily avoids having these turds in the first place and IIRC NetBSD
simply doesn't have means for sharing descriptor tables. Should such means
appear it would be vulnerable as well. Dnotify is Linux-only, er, entity
(as in "non sunt multiplicandam"). I haven't looked at Solaris and I couldn't
care less about GNU Turd.

In all cases vulnerablities are local, with impact ranging from
user-triggered panic to rather unpleasant privelege escalations (e.g.
"any user can send an arbitrary signal to arbitrary process" in case of
dnotify, etc.)

The nature of mistaken assumption is exactly the same in all cases.
An object is associated with vnode/dentry/inode/whatnot and since it's
destroyed on any close(), it is assumed that we shall be guaranteed that
such objects will not be able to outlive the opened file they are associated
with or the process creating them. It leads to the following nastiness:
A and B share descriptor table.
A: fcntl(fd, ...) trying to create such object; it resolves descriptor to
opened file, pins it down for the duration of operation and blocks
somewhere in course of creating the object.
B: close(fd) evicts opened file from descriptor table. It finds no objects
to be destroyed.
A: completes creation of object, associates it with filesystem object and
releases the temporary hold it had on opened file.

At that point we have an obvious leak and slightly less obvious attack vector.
If no other descriptors in the descriptor table of A and B used to refer to
the same file, the object will not be destroyed since there will be nothing
that could decide to destroy it. Unfortunately, it's worse than just a leak.
These objects are supposed to be destroyed before the end of life of opened
file. As the result, nobody bothers to have them affect refcounts on the
file/vnode/dentry/inode/whatever. That's perfectly fine - the proper fix is
to have fcntl() verify that descriptor still resolves to the same file before
completing its work and destroy the object if it doesn't. You don't need to
play with refcounts for that. However, without that fix we have a leak that
leads to more than just an undead object - it's an undead object containing
references to filesystem object that might be reused and to task_struct/proc/
whatever you call it, again with the possibility of reuse.

Getting from that point to the attack is a matter of simple (really simple)
RTFS. Details obviously depend on what the kernel in question is doing to
these objects, but with that kind of broken assertions it's really not hard
to come up with exploitable holes.

Now, let us look at the history:
* POSIX locks support predates shared descriptor tables; the holes
in question opened as soon as clone(2)/rfork(2) had been merged into a kernel
and grown support for shared descriptor tables. For Linux it's 1.3.22 (Sep
1995), for OpenBSD it's a bit before 2.0 (Jan 1996) for FreeBSD - 2.2 (Feb
1996, from OpenBSD).
* In 2002 dnotify had grown the same semantics (Linux-only thing,
2.5.15, soon backported to 2.4.19). Same kind of race.
* In 2003 FreeBSD folks had found and fixed their instance of that bug;
commit message:
"Avoid file lock leakage when linuxthreads port or rfork is used:
- Mark the process leader as having an advisory lock
- Check if process leader is marked as having advisory lock when
closing file
- Check that file is still open after lock has been obtained
- Don't allow file descriptor table sharing between processes
with different leaders"
"Check that file is still open" bit refers to that race. I have no idea
whether they'd realized that what they'd closed had been locally exploitable
or not.
* In 2005 Peter Staubach had noticed the Linux analog of that sucker.
The fix had been along the same lines as FreeBSD one, but in case of Linux
we had extra fun with SMP ordering. Peter had missed that and his patch
left a hard to hit remnant of the race. His commit message is rather long;
it starts with
[PATCH] stale POSIX lock handling

I believe that there is a problem with the handling of POSIX locks, which
the attached patch should address.

The problem appears to be a race between fcntl(2) and close(2). A
multithreaded application could close a file descriptor at the same time as
it is trying to acquire a lock using the same file descriptor. I would
suggest that that multithreaded application is not providing the proper
synchronization for itself, but the OS should still behave correctly.
...
I'm 100% sure that in this case the vulnerability had _not_ been realized.
Bogus behaviour had been noticed and (mostly) fixed, but implications had
been missed, along with the fact that the same scenario had played out in
dnotify.
* This April I'd caught dnotify hole during code audit. The fix
had been trivial enough and seeing that impact had been fairly nasty (any
user could send any signal to any process, among other things) I'd decided
to play along with "proper mechanisms". Meaning vendor-sec. _Bad_ error
in judgement; the damn thing had not improved since I'd unsubscribed from
that abortion. A trivial patch, obviously local to one function and obviously
not modifying behaviour other than in case when existing tree would've screwed
itself. Not affecting any headers. Not affecting any data structures.
_Obviously_ not affecting any 3rd-party code - not even on binary level.
IOW, as safe as it ever gets.
Alas. The usual shite had played out and we had a *MONTH-LONG*
embargo. I would like to use this opportunity to offer a whole-hearted
"fuck you" to that lovely practice and to vendor-sec in general.
* Very soon after writing the first version of a fix I started
wondering if POSIX locks had the same hole - the same kind of semantics
had invited the same kind of race there (eviction of dnotify entries and
eviction of POSIX locks are called in the same place in close(2)). Current
tree appeared to be OK, checking the history had shown Peter's patch.
A bit after that I'd noticed insufficient locking in dnotify patch, fixed
that. Checking for similar problems in POSIX locks counterpart of that crap
had found the SMP ordering bug that remained there.
* 2.6 -> 2.4 backport had uncovered another interesting thing -
Peter's patch did _not_ make it to 2.4 3 years ago.
* Checking OpenBSD (only now - I didn't get around to that back in
April) shows that the same hole is alive and well there.

Note that
* OpenBSD folks hadn't picked the fix from FreeBSD ones, even though
the FreeBSD version had been derived from OpenBSD one. Why? At a guess, the
commit message had been less than noticable. Feel free to toss yourself off
screaming "coverup" if you are so inclined; I don't swing that way...
* Initial Linux fix _definitely_ had missed security implications
*and* realization that somebody else might suffer from the same problem.
FVO "somebody" including Linux itself, not to mention *BSD.
* Even when the problem had been realized for what it had been in
Linux, *BSD potential issues hadn't registered. Again, the same wunch
of bankers is welcome to scream "coverup", but in this case we even have
the bleeding CVEs.
* CVEs or no CVEs, OpenBSD folks hadn't picked that one.
* Going to vendor-sec is a mistake I won't repeat any time soon and
I would strongly recommend everybody else to stay the hell away from that
morass. It creates inexcusable delays, bounds you to confidentiality and,
let's face it, happens to be the prime infiltration target for zero-day
exploit traders. In _this_ case exploit had been local-only. Anything more
interesting...

So where does that leave us? Bugger if I know... FWIW, I would rather see
implications thought about *and* mentioned in the changelogs. OTOH, the
above shows the real-world cases when breakage hadn't even been realized to
be security-significant. Obviously broken behaviour (leak, for example)
gets spotted and fixed. Fix looks obviously sane, bug it deals with -
obviously real and worth fixing, so into a tree it goes... IOW, one _can't_
rely on having patches that close security holes marked as such. For that
the authors have to notice that themselves in the first place. OTTH, nothing
is going to convince the target audience of grsec, er, gentlemen that we are
not a massive conspiracy anyway, the tinfoil brigade being what it is...

Subject: Re: [stable] Linux 2.6.25.10 (resume)

--- SecurityBugs.orig 2008-07-16 23:46:09.000000000 -0300
+++ SecurityBugs 2008-07-21 07:28:01.000000000 -0300
@@ -1,7 +1,9 @@
-Linux kernel developers take security very seriously. As such, we'd
-like to know when a security bug is found so that it can be fixed and
-disclosed as quickly as possible. Please report security bugs to the
-Linux kernel security team.
+Linux kernel developers take security very seriously, in exactly the
+same way we do with any other bugs. As such, we'd like to know when
+a security bug is found so that it can be fixed as soon as possible by
+the experts in this portion of the kernel code.
+
+Please report security bugs to the Linux kernel security team.

1) Contact

@@ -14,23 +16,26 @@
As it is with any bug, the more information provided the easier it
will be to diagnose and fix. Please review the procedure outlined in
REPORTING-BUGS if you are unclear about what information is helpful.
-Any exploit code is very helpful and will not be released without
-consent from the reporter unless it has already been made public.
+Any exploit code is very helpful and will not be released by our team
+unless already made public. The exploit code may be shared with third
+parties to facilitate a fix or to verify the vulnerability.

2) Disclosure

The goal of the Linux kernel security team is to work with the
bug submitter to bug resolution as well as disclosure. We prefer
-to fully disclose the bug as soon as possible. It is reasonable to
+to not disclose the bug, since we believe any kind of bug deserves the
+same attention and will be quickly patched. It is reasonable to
delay disclosure when the bug or the fix is not yet fully understood,
the solution is not well-tested or for vendor coordination. However, we
expect these delays to be short, measurable in days, not weeks or months.
A disclosure date is negotiated by the security team working with the
-bug submitter as well as vendors. However, the kernel security team
-holds the final say when setting a disclosure date. The timeframe for
-disclosure is from immediate (esp. if it's already publically known)
-to a few weeks. As a basic default policy, we expect report date to
-disclosure date to be on the order of 7 days.
+bug submitter as well as vendors if the submitter wants to disclose.
+However, the kernel security team holds the final say when setting a
+disclosure date. The timeframe for disclousure is from immediate (esp. if
+it's already publically known) to a few weeks. As a basic default policy,
+we expect report date to disclosure (if the submitter requires disclosure)
+to be on the order of 7 days.

3) Non-disclosure agreements


Attachments:
SecurityBugs.patch (2.63 kB)
Subject: Re: [stable] Linux 2.6.25.10 (resume)

Greg KH escreveu:
> On Fri, Jul 18, 2008 at 11:07:45AM -0300, Rodrigo Rubira Branco (BSDaemon) wrote:
>
>> --- SecurityBugs.orig 2008-07-16 23:46:09.000000000 -0300
>> +++ SecurityBugs 2008-07-17 14:58:32.000000000 -0300
>> @@ -1,7 +1,7 @@
>> -Linux kernel developers take security very seriously. As such, we'd
>> -like to know when a security bug is found so that it can be fixed and
>> -disclosed as quickly as possible. Please report security bugs to the
>> -Linux kernel security team.
>> +Linux kernel developers take security very seriously, in exactly the
>> +same way we do with any other bugs. As such, we'd like to know when
>> +a security bug is found so that it can be fixed as soon as possible.
>> +Please report security bugs to the Linux kernel security team.
>>
>
> I guess what is getting everyone's panties all in a bind is the term
> "disclosed", right? Why not just drop this word from the sentence
> instead of rewording it so much?
>
No, it's not ;) The problem is the policy of normal bugs = security
bugs. If it's clear in the documentation, it will make people who need
to backport patches aware of that and then they will need to care by
themselves.

The idea of start a project to keep track of that is not bad, but the
problem is it's almost impossible to keep track of everything. The
kernel developers knows about the bugs when they correct it, so just
note as a security problem is not a big issue (but we already accepted
it will not be done). Maybe if you guys accept to note it as a security
issue at least to a group of people who can work on document it, it's
cool to me ;)
>
>> @@ -14,23 +14,24 @@
>> As it is with any bug, the more information provided the easier it
>> will be to diagnose and fix. Please review the procedure outlined in
>> REPORTING-BUGS if you are unclear about what information is helpful.
>> -Any exploit code is very helpful and will not be released without
>> -consent from the reporter unless it has already been made public.
>> +Any exploit code is very helpful and will not be released.
>>
>
> I don't see why this needs to be changed, sometimes we do release
> exploit code to third parties that ask us nicely and the reporter allows
> us to.
>
Great, I agreed with that and already sent another version changing this
sentence.

>
>> 2) Disclosure
>>
>> The goal of the Linux kernel security team is to work with the
>> bug submitter to bug resolution as well as disclosure. We prefer
>> -to fully disclose the bug as soon as possible.
>>
>
> Ah, again, it's the "fully disclose" that is causing panties to ride
> high. And again, we are disclosing the bug with the real fix and the
> code in question. We just seem to differ on what people consider
> "fully" it seems. I think the people liking that term these days
> consider that you must release exploit and other detailed information.
>
>
No, that's not true... We just want a sentence in the fix saying a
security issue have been fixed. Not a detailed explanation, so the
developers don't need to wast important time trying to understand
security problems. The issue is that they know it's a security fix but
they don't put that and sometimes they remove any reference to that ;)
> I disagree with this and feel that our current policy of fixing bugs and
> releasing full code is pretty much the same thing as we are doing today,
> although I can understand the confusion. How about this rewording of
> the sentance instead:
>
> We prefer to fix and provide an update for the bug as soon as
> possible.
>
> So a simple 1 line change should be enough to stem this kind of argument
> in the future, right?
>
I really feel more confortable with the new version that I just sent -
it's more cleaver about how it's handled... Please, give-me your
insights on it too...


cya,


Rodrigo (BSDaemon).

Subject: Re: [stable] Linux 2.6.25.10 (resume)

Al Viro escreveu:
> On Sat, Jul 19, 2008 at 03:13:43PM -0700, Greg KH wrote:
>
>
>> I disagree with this and feel that our current policy of fixing bugs and
>> releasing full code is pretty much the same thing as we are doing today,
>> although I can understand the confusion. How about this rewording of
>> the sentance instead:
>>
>> We prefer to fix and provide an update for the bug as soon as
>> possible.
>>
>> So a simple 1 line change should be enough to stem this kind of argument
>> in the future, right?
>>
>
> Not quite... OK, here's a story that might serve as a model
> of all that crap - it certainly runs afoul of a bunch of arguments on
> all sides of that.
>
Ual, crap? :-)

> We all know that POSIX locks suck by design, in particular where it
> deals with close(2) semantics. "$FOO is associated with process P having
> a descriptor refering to opened file F, $FOO disappears when any of such
> descriptors get removed" is bloody inconvenient in a lot of respects. It
> also turns out to invite very similar kind of wrong assumptions in all
> implementation that have to deal with descriptor tables being possibly
> shared. So far the victims include:
> * FreeBSD POSIX locks; used to be vulnerable, fixed.
> * OpenBSD POSIX locks; vulnerable.
> * Linux POSIX locks and dnotify entries; used to be vulnerable, fixed.
> Plan9 happily avoids having these turds in the first place and IIRC NetBSD
> simply doesn't have means for sharing descriptor tables. Should such means
> appear it would be vulnerable as well. Dnotify is Linux-only, er, entity
> (as in "non sunt multiplicandam"). I haven't looked at Solaris and I couldn't
> care less about GNU Turd.
>
> In all cases vulnerablities are local, with impact ranging from
> user-triggered panic to rather unpleasant privelege escalations (e.g.
> "any user can send an arbitrary signal to arbitrary process" in case of
> dnotify, etc.)
>
> The nature of mistaken assumption is exactly the same in all cases.
> An object is associated with vnode/dentry/inode/whatnot and since it's
> destroyed on any close(), it is assumed that we shall be guaranteed that
> such objects will not be able to outlive the opened file they are associated
> with or the process creating them. It leads to the following nastiness:
> A and B share descriptor table.
> A: fcntl(fd, ...) trying to create such object; it resolves descriptor to
> opened file, pins it down for the duration of operation and blocks
> somewhere in course of creating the object.
> B: close(fd) evicts opened file from descriptor table. It finds no objects
> to be destroyed.
> A: completes creation of object, associates it with filesystem object and
> releases the temporary hold it had on opened file.
>
> At that point we have an obvious leak and slightly less obvious attack vector.
> If no other descriptors in the descriptor table of A and B used to refer to
> the same file, the object will not be destroyed since there will be nothing
> that could decide to destroy it. Unfortunately, it's worse than just a leak.
> These objects are supposed to be destroyed before the end of life of opened
> file. As the result, nobody bothers to have them affect refcounts on the
> file/vnode/dentry/inode/whatever. That's perfectly fine - the proper fix is
> to have fcntl() verify that descriptor still resolves to the same file before
> completing its work and destroy the object if it doesn't. You don't need to
> play with refcounts for that. However, without that fix we have a leak that
> leads to more than just an undead object - it's an undead object containing
> references to filesystem object that might be reused and to task_struct/proc/
> whatever you call it, again with the possibility of reuse.
>
> Getting from that point to the attack is a matter of simple (really simple)
> RTFS. Details obviously depend on what the kernel in question is doing to
> these objects, but with that kind of broken assertions it's really not hard
> to come up with exploitable holes.
>
> Now, let us look at the history:
> * POSIX locks support predates shared descriptor tables; the holes
> in question opened as soon as clone(2)/rfork(2) had been merged into a kernel
> and grown support for shared descriptor tables. For Linux it's 1.3.22 (Sep
> 1995), for OpenBSD it's a bit before 2.0 (Jan 1996) for FreeBSD - 2.2 (Feb
> 1996, from OpenBSD).
> * In 2002 dnotify had grown the same semantics (Linux-only thing,
> 2.5.15, soon backported to 2.4.19). Same kind of race.
> * In 2003 FreeBSD folks had found and fixed their instance of that bug;
> commit message:
> "Avoid file lock leakage when linuxthreads port or rfork is used:
> - Mark the process leader as having an advisory lock
> - Check if process leader is marked as having advisory lock when
> closing file
> - Check that file is still open after lock has been obtained
> - Don't allow file descriptor table sharing between processes
> with different leaders"
> "Check that file is still open" bit refers to that race. I have no idea
> whether they'd realized that what they'd closed had been locally exploitable
> or not.
> * In 2005 Peter Staubach had noticed the Linux analog of that sucker.
> The fix had been along the same lines as FreeBSD one, but in case of Linux
> we had extra fun with SMP ordering. Peter had missed that and his patch
> left a hard to hit remnant of the race. His commit message is rather long;
> it starts with
> [PATCH] stale POSIX lock handling
>
> I believe that there is a problem with the handling of POSIX locks, which
> the attached patch should address.
>
> The problem appears to be a race between fcntl(2) and close(2). A
> multithreaded application could close a file descriptor at the same time as
> it is trying to acquire a lock using the same file descriptor. I would
> suggest that that multithreaded application is not providing the proper
> synchronization for itself, but the OS should still behave correctly.
> ...
> I'm 100% sure that in this case the vulnerability had _not_ been realized.
> Bogus behaviour had been noticed and (mostly) fixed, but implications had
> been missed, along with the fact that the same scenario had played out in
> dnotify.
>
That's common... We know that will occur from time to time and we are
not discussing about that... Just about already known security issues
being silently fixed without any mention.
> * This April I'd caught dnotify hole during code audit. The fix
> had been trivial enough and seeing that impact had been fairly nasty (any
> user could send any signal to any process, among other things) I'd decided
> to play along with "proper mechanisms". Meaning vendor-sec. _Bad_ error
> in judgement; the damn thing had not improved since I'd unsubscribed from
> that abortion. A trivial patch, obviously local to one function and obviously
> not modifying behaviour other than in case when existing tree would've screwed
> itself. Not affecting any headers. Not affecting any data structures.
> _Obviously_ not affecting any 3rd-party code - not even on binary level.
> IOW, as safe as it ever gets.
> Alas. The usual shite had played out and we had a *MONTH-LONG*
> embargo. I would like to use this opportunity to offer a whole-hearted
> "fuck you" to that lovely practice and to vendor-sec in general.
>
huahuahuahuahuhuahuahu, tks for share your felling about that... Maybe
the wrong people are dealing with that... What do you think?
> * Very soon after writing the first version of a fix I started
> wondering if POSIX locks had the same hole - the same kind of semantics
> had invited the same kind of race there (eviction of dnotify entries and
> eviction of POSIX locks are called in the same place in close(2)). Current
> tree appeared to be OK, checking the history had shown Peter's patch.
> A bit after that I'd noticed insufficient locking in dnotify patch, fixed
> that. Checking for similar problems in POSIX locks counterpart of that crap
> had found the SMP ordering bug that remained there.
> * 2.6 -> 2.4 backport had uncovered another interesting thing -
> Peter's patch did _not_ make it to 2.4 3 years ago.
> * Checking OpenBSD (only now - I didn't get around to that back in
> April) shows that the same hole is alive and well there.
>
> Note that
> * OpenBSD folks hadn't picked the fix from FreeBSD ones, even though
> the FreeBSD version had been derived from OpenBSD one. Why? At a guess, the
> commit message had been less than noticable. Feel free to toss yourself off
> screaming "coverup" if you are so inclined; I don't swing that way...
>
I ever used this word... I know others did anyway.
> * Initial Linux fix _definitely_ had missed security implications
> *and* realization that somebody else might suffer from the same problem.
> FVO "somebody" including Linux itself, not to mention *BSD.
>
That's the main problem of 'hidden' security bugs... Hidden here must be
understood as: normal bugs = security bugs
> * Even when the problem had been realized for what it had been in
> Linux, *BSD potential issues hadn't registered. Again, the same wunch
> of bankers is welcome to scream "coverup", but in this case we even have
> the bleeding CVEs.
> * CVEs or no CVEs, OpenBSD folks hadn't picked that one.
>
Yeah, they don't pay much attention to other projects I they should...
That's why I always talk about 'copy and paste security bugs' as a real
problem in open-source projects... Because the original code may be
fixed and the 'copied' one not ;)
> * Going to vendor-sec is a mistake I won't repeat any time soon and
> I would strongly recommend everybody else to stay the hell away from that
> morass. It creates inexcusable delays, bounds you to confidentiality and,
>
Interesting... every people involved in this discussion told us to
change our behaviour and try to improve things instead of be against
it... Why not change people involved in the vendor-sec in some way? I
have no idea who are there, but I'm sure it can be improved since I
already worked with many vendors to coordinate vuln-disclousure and had
no problems.

I'm sure the vendors involved are caring about 'marketing' and things
like that, that's why there is a delay... They need to learn from others
mistakes, like Microsoft. The company now really knows how to deal with
security problems.
> let's face it, happens to be the prime infiltration target for zero-day
> exploit traders. In _this_ case exploit had been local-only. Anything more
> interesting...
>
That's true. Mainly because it takes longer to have a fix... I agree
with the fix asap idea, just don't agree with 'change the message in the
patch to not apper to be a security bug been fixed'.
> So where does that leave us? Bugger if I know... FWIW, I would rather see
> implications thought about *and* mentioned in the changelogs. OTOH, the
> above shows the real-world cases when breakage hadn't even been realized to
> be security-significant.
We ever said it's different otherwise...
> Obviously broken behaviour (leak, for example)
> gets spotted and fixed. Fix looks obviously sane, bug it deals with -
> obviously real and worth fixing, so into a tree it goes... IOW, one _can't_
> rely on having patches that close security holes marked as such.
Sometimes no one will figure out that patch closed a security issue...
We are talking in this thread about patches that are well-known to fix
security holes... As the bugzilla explicitly says about a security
problem and in the commit nothing mention that.
> For that
> the authors have to notice that themselves in the first place. OTTH, nothing
> is going to convince the target audience of grsec, er, gentlemen that we are
> not a massive conspiracy anyway, the tinfoil brigade being what it is...
>

There is no target audience involved here... There is no 'marketing' or
'media circus'... It's just a try to improve things.


cya,


Rodrigo (BSDaemon).


PS: Just my personal thoughts, not related to the company that I work for.

2008-07-23 04:34:14

by Greg KH

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Mon, Jul 21, 2008 at 09:48:13PM -0300, Rodrigo Rubira Branco (BSDaemon) wrote:
> Alan Cox escreveu:
>>> @@ -1,7 +1,7 @@
>>> -Linux kernel developers take security very seriously. As such, we'd
>>> -like to know when a security bug is found so that it can be fixed and
>>> -disclosed as quickly as possible. Please report security bugs to the
>>> -Linux kernel security team.
>>> +Linux kernel developers take security very seriously, in exactly the
>>> +same way we do with any other bugs. As such, we'd like to know when +a
>>> security bug is found so that it can be fixed as soon as possible.
>>> +Please report security bugs to the Linux kernel security team.
>>>
>>
>> NAK this. If the fix is not clear and the bug not too serious it is better
>> to disclose it than fail to fix it. The security team does not usually fix
>> the
>> bugs, the experts in the various bits of code do.
>>
> ACK ;) Changed the sentence. Tks.
>
>>> -Any exploit code is very helpful and will not be released without
>>> -consent from the reporter unless it has already been made public.
>>> +Any exploit code is very helpful and will not be released.
>>>
>>
>> NAK this too. If someone releases an exploit publically or it leaks we
>> want to be able to freely share it too. Your proposal would mean any but
>> those dumb enough to agree to this could share it. That is why the unless
>> made
>> public is part of every generic NDA document on the planet.
>>
> Agreed. Changed the sentence. Tks.
>> The rest needs Linus to return from holiday for discussion and that'll
>> be a week or two. In the meantime you might want to define "disclose" as
>> I don't think we all agree on what it means as you've not defined who is
>> and
>> isn't the linux security team and/or its helpers.
> Cool.

> --- SecurityBugs.orig 2008-07-16 23:46:09.000000000 -0300
> +++ SecurityBugs 2008-07-21 07:28:01.000000000 -0300
> @@ -1,7 +1,9 @@
> -Linux kernel developers take security very seriously. As such, we'd
> -like to know when a security bug is found so that it can be fixed and
> -disclosed as quickly as possible. Please report security bugs to the
> -Linux kernel security team.
> +Linux kernel developers take security very seriously, in exactly the
> +same way we do with any other bugs. As such, we'd like to know when
> +a security bug is found so that it can be fixed as soon as possible by
> +the experts in this portion of the kernel code.
> +
> +Please report security bugs to the Linux kernel security team.

You have trailing spaces in your patch, please run all patches through
scripts/checkpatch.pl to catch stuff like this.

> @@ -14,23 +16,26 @@
> As it is with any bug, the more information provided the easier it
> will be to diagnose and fix. Please review the procedure outlined in
> REPORTING-BUGS if you are unclear about what information is helpful.
> -Any exploit code is very helpful and will not be released without
> -consent from the reporter unless it has already been made public.
> +Any exploit code is very helpful and will not be released by our team
> +unless already made public. The exploit code may be shared with third
> +parties to facilitate a fix or to verify the vulnerability.

Your two changed sentances are contradictory.
What is wrong with the original wording?

> 2) Disclosure
>
> The goal of the Linux kernel security team is to work with the
> bug submitter to bug resolution as well as disclosure. We prefer
> -to fully disclose the bug as soon as possible. It is reasonable to
> +to not disclose the bug, since we believe any kind of bug deserves the
> +same attention and will be quickly patched. It is reasonable to

No, again, you are using the word "disclose" in an odd way here.

Of course the bug is "disclosed" in that it is made public, "here's a
fix for foo breaking".

So this wording change is not right.

> delay disclosure when the bug or the fix is not yet fully understood,
> the solution is not well-tested or for vendor coordination. However, we
> expect these delays to be short, measurable in days, not weeks or months.
> A disclosure date is negotiated by the security team working with the
> -bug submitter as well as vendors. However, the kernel security team
> -holds the final say when setting a disclosure date. The timeframe for
> -disclosure is from immediate (esp. if it's already publically known)
> -to a few weeks. As a basic default policy, we expect report date to
> -disclosure date to be on the order of 7 days.
> +bug submitter as well as vendors if the submitter wants to disclose.

Again, trailing spaces.

What was wrong with the original wording?

thanks,

greg k-h

2008-07-23 12:11:34

by PaX Team

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On 22 Jul 2008 at 21:27, Greg KH wrote:

> > 2) Disclosure
> >
> > The goal of the Linux kernel security team is to work with the
> > bug submitter to bug resolution as well as disclosure. We prefer
> > -to fully disclose the bug as soon as possible. It is reasonable to
> > +to not disclose the bug, since we believe any kind of bug deserves the
> > +same attention and will be quickly patched. It is reasonable to
>
> No, again, you are using the word "disclose" in an odd way here.
>
> Of course the bug is "disclosed" in that it is made public, "here's a
> fix for foo breaking".

it's apparently not true when foo = "kernel's security model", hence the
suggested change to reflect reality.

Subject: Re: [stable] Linux 2.6.25.10 (resume)

On Wed, 23 Jul 2008, [email protected] wrote:
> On 22 Jul 2008 at 21:27, Greg KH wrote:
> > > 2) Disclosure
> > >
> > > The goal of the Linux kernel security team is to work with the
> > > bug submitter to bug resolution as well as disclosure. We prefer
> > > -to fully disclose the bug as soon as possible. It is reasonable to
> > > +to not disclose the bug, since we believe any kind of bug deserves the
> > > +same attention and will be quickly patched. It is reasonable to
> >
> > No, again, you are using the word "disclose" in an odd way here.
> >
> > Of course the bug is "disclosed" in that it is made public, "here's a
> > fix for foo breaking".
>
> it's apparently not true when foo = "kernel's security model", hence the
> suggested change to reflect reality.

I heavily suggest using something else than "disclose".

For the security community, "disclose" doesn't mean you have the source code
for the buggy code and the source code for the fix. It means you have the
information that it is a "foo = kernel's security model" bug, and a
description of the consequences of the bug for foo (the security model).

This is NOT what "disclose" means for the Linux kernel, right now. Here,
"disclose" means "you know there is a bug, you have the code, you have the
bug fix". But you don't know that "foo = kernel's security bug", or the
consequences of the bug for the security model.

So just use another word, or properly qualify WHAT is going to be disclosed,
(and in this case, WHAT is not going to be *usually* disclosed).

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2008-07-23 14:57:36

by PaX Team

[permalink] [raw]
Subject: Re: [stable] Linux 2.6.25.10 (resume)

On 23 Jul 2008 at 11:31, Henrique de Moraes Holschuh wrote:

> On Wed, 23 Jul 2008, [email protected] wrote:
> > it's apparently not true when foo = "kernel's security model", hence the
> > suggested change to reflect reality.
>
> I heavily suggest using something else than "disclose".
>
> For the security community, "disclose" doesn't mean you have the source code
> for the buggy code and the source code for the fix. It means you have the
> information that it is a "foo = kernel's security model" bug, and a
> description of the consequences of the bug for foo (the security model).
>
> This is NOT what "disclose" means for the Linux kernel, right now. Here,
> "disclose" means "you know there is a bug, you have the code, you have the
> bug fix". But you don't know that "foo = kernel's security bug", or the
> consequences of the bug for the security model.

i think you misunderstood the whole thread here ;). we were explicitly
talking about bugs where the kernel devs *knew* they were fixing one
with an impact on security yet they chose not to say so.

determining whether a bug is a security one is a whole different kettle
of fish, that was not the topic here at all.

IOW, Documentation/SecurityBugs talks about bugs where the security impact
is known, not about bugs in general where such determination has yet to be
done.

> So just use another word, or properly qualify WHAT is going to be disclosed,
> (and in this case, WHAT is not going to be *usually* disclosed).