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

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.

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

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

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

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

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

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

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

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

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

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

Hi Greg;

01 Tem 2008 Sal tarihinde, Greg KH şunları yazmıştı:

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!

Eugene Teo posted the patch to fix that.

Alan

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

Hi;

02 Tem 2008 Çar tarihinde, Greg KH şunları yazmıştı:

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.

Sorry for your time...

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!

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

{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

Greg KH escreveu:

Attached ;) Please, let me know what do you think.

For free? Hum, let's forget that you work as a linux developer... we are also trying to contribute in some way for free...

P.S: It's obvious that my opinions are mine, not of my employer ;)

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

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

Willy Tarreau escreveu:

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

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.

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

Alan Cox escreveu:

ACK ;) Changed the sentence. Tks.

Agreed. Changed the sentence. Tks.

Cool.

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

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

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.

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

Al Viro escreveu:

Ual, crap? :-)

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.

huahuahuahuahuhuahuahu, tks for share your felling about that... Maybe the wrong people are dealing with that... What do you think?

I ever used this word... I know others did anyway.

That's the main problem of 'hidden' security bugs... Hidden here must be understood as: normal bugs = security bugs

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

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.

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'.

We ever said it's different otherwise...

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.

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.

Greg KH escreveu:

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

Great, I agreed with that and already sent another version changing this sentence.

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

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.

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

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

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 ]

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

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

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.

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

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

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.

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

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

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)

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

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

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

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

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

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.

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

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

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.

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

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

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