Skip to contentSkip to conversation

-stable review

PATCH In review Greg KH 11 participants 27 messages Jul 13, 2005 – Jul 24, 2005 1 message not in the archive

Subsystems (15)
lore
Share & tools
lore

Greg KH

author
#0 lore raw

cover -stable review

This is the start of the stable review cycle for the 2.6.12.3 release. There are 11 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let us know. If anyone is a maintainer of the proper subsystem, and wants to add a signed-off-by: line to the patch, please respond with it.

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 Friday, July 15, 20:00:00 UTC 2005. Anything received after that time, might be too late.

thanks,

the -stable release team

author
#1 lore raw

PATCH 2/11 : revert nf_reset change

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

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

[NETFILTER]: Revert nf_reset change

Revert the nf_reset change that caused so much trouble, drop conntrack references manually before packets are queued to packet sockets.

Adapted for 2.6.12 by Daniel Drake <[email protected]>

---
 net/ipv4/ip_output.c                         |    3 ---
 net/ipv4/netfilter/ip_conntrack_standalone.c |    7 +++++++
 net/packet/af_packet.c                       |    6 ++++++
 3 files changed, 13 insertions(+), 3 deletions(-)
net/ipv4/ip_output.c +0 -3
@@ -111,7 +111,6 @@
 #ifdef CONFIG_NETFILTER_DEBUG
 	nf_debug_ip_loopback_xmit(newskb);
 #endif
-	nf_reset(newskb);
 	netif_rx(newskb);
 	return 0;
 }
@@ -196,8 +195,6 @@
 	nf_debug_ip_finish_output2(skb);
 #endif /*CONFIG_NETFILTER_DEBUG*/
 
-	nf_reset(skb);
-
 	if (hh) {
 		int hh_alen;
 
net/ipv4/netfilter/ip_conntrack_standalone.c +7 -0
@@ -432,6 +432,13 @@
 				        const struct net_device *out,
 				        int (*okfn)(struct sk_buff *))
 {
+#if !defined(CONFIG_IP_NF_NAT) && !defined(CONFIG_IP_NF_NAT_MODULE)
+	/* Previously seen (loopback)?  Ignore.  Do this before
+           fragment check. */
+	if ((*pskb)->nfct)
+		return NF_ACCEPT;
+#endif
+
 	/* Gather fragments. */
 	if ((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET)) {
 		*pskb = ip_ct_gather_frags(*pskb,
net/packet/af_packet.c +6 -0
@@ -274,6 +274,9 @@
 	dst_release(skb->dst);
 	skb->dst = NULL;
 
+	/* drop conntrack reference */
+	nf_reset(skb);
+
 	spkt = (struct sockaddr_pkt*)skb->cb;
 
 	skb_push(skb, skb->data-skb->mac.raw);
@@ -517,6 +520,9 @@
 	dst_release(skb->dst);
 	skb->dst = NULL;
 
+	/* drop conntrack reference */
+	nf_reset(skb);
+
 	spin_lock(&sk->sk_receive_queue.lock);
 	po->stats.tp_packets++;
 	__skb_queue_tail(&sk->sk_receive_queue, skb);

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw

PATCH 1/11 If ACPI doesn't find an irq listed, don't accept 0 as a valid PCI irq.

[resend to get threading correct, sorry]

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

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

If ACPI doesn't find an irq listed, don't accept 0 as a valid PCI irq. That zero just means that nothing else found any irq information either.

Fixes http://bugme.osdl.org/show_bug.cgi?id=4824

---
 drivers/acpi/pci_irq.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/acpi/pci_irq.c +1 -1
@@ -433,7 +433,7 @@
 		printk(KERN_WARNING PREFIX "PCI Interrupt %s[%c]: no GSI",
 			pci_name(dev), ('A' + pin));
 		/* Interrupt Line values above 0xF are forbidden */
-		if (dev->irq >= 0 && (dev->irq <= 0xF)) {
+		if (dev->irq > 0 && (dev->irq <= 0xF)) {
 			printk(" - using IRQ %d\n", dev->irq);
 			acpi_register_gsi(dev->irq, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW);
 			return_VALUE(0);

-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual core and dual graphics technology at this free one hour event hosted by HP, AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar

author
#4 lore raw

PATCH 3/11 ppc32: stop misusing ntps time_offset value

In reply to Greg KH

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

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

From: john stultz <[email protected]>

As part of my timeofday rework, I've been looking at the NTP code and I noticed that the PPC architecture is apparently misusing the NTP's time_offset (it is a terrible name!) value as some form of timezone offset.

This could cause problems when time_offset changed by the NTP code. This patch changes the PPC code so it uses a more clear local variable:
timezone_offset.

---
 arch/ppc/kernel/time.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)
arch/ppc/kernel/time.c +8 -5
@@ -89,6 +89,9 @@
 
 extern unsigned long wall_jiffies;
 
+/* used for timezone offset */
+static long timezone_offset;
+
 DEFINE_SPINLOCK(rtc_lock);
 
 EXPORT_SYMBOL(rtc_lock);
@@ -170,7 +173,7 @@
 		     xtime.tv_sec - last_rtc_update >= 659 &&
 		     abs((xtime.tv_nsec / 1000) - (1000000-1000000/HZ)) < 500000/HZ &&
 		     jiffies - wall_jiffies == 1) {
-		  	if (ppc_md.set_rtc_time(xtime.tv_sec+1 + time_offset) == 0)
+		  	if (ppc_md.set_rtc_time(xtime.tv_sec+1 + timezone_offset) == 0)
 				last_rtc_update = xtime.tv_sec+1;
 			else
 				/* Try again one minute later */
@@ -286,7 +289,7 @@
 	unsigned old_stamp, stamp, elapsed;
 
         if (ppc_md.time_init != NULL)
-                time_offset = ppc_md.time_init();
+                timezone_offset = ppc_md.time_init();
 
 	if (__USE_RTC()) {
 		/* 601 processor: dec counts down by 128 every 128ns */
@@ -331,10 +334,10 @@
 	set_dec(tb_ticks_per_jiffy);
 
 	/* If platform provided a timezone (pmac), we correct the time */
-        if (time_offset) {
-		sys_tz.tz_minuteswest = -time_offset / 60;
+        if (timezone_offset) {
+		sys_tz.tz_minuteswest = -timezone_offset / 60;
 		sys_tz.tz_dsttime = 0;
-		xtime.tv_sec -= time_offset;
+		xtime.tv_sec -= timezone_offset;
         }
         set_normalized_timespec(&wall_to_monotonic,
                                 -xtime.tv_sec, -xtime.tv_nsec);
author
#5 lore raw

PATCH 4/11 fix Shaper driver lossage in 2.6.12

In reply to Greg KH

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

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

[SHAPER]: Switch to spinlocks.

Dave, you were right and the sleeping locks in shaper were broken. Markus Kanet noticed this and also tested the patch below that switches locking to spinlocks.

---
 drivers/net/shaper.c      |   42 ++++++++++++++++--------------------------
 include/linux/if_shaper.h |    2 +-
 2 files changed, 17 insertions(+), 27 deletions(-)
drivers/net/shaper.c +16 -26
@@ -135,10 +135,8 @@
 {
 	struct shaper *shaper = dev->priv;
  	struct sk_buff *ptr;
-   
-	if (down_trylock(&shaper->sem))
-		return -1;
-
+
+	spin_lock(&shaper->lock);
  	ptr=shaper->sendq.prev;
  	
  	/*
@@ -232,7 +230,7 @@
                 shaper->stats.collisions++;
  	}
 	shaper_kick(shaper);
-	up(&shaper->sem);
+	spin_unlock(&shaper->lock);
  	return 0;
 }
 
@@ -271,11 +269,9 @@
 {
 	struct shaper *shaper = (struct shaper *)data;
 
-	if (!down_trylock(&shaper->sem)) {
-		shaper_kick(shaper);
-		up(&shaper->sem);
-	} else
-		mod_timer(&shaper->timer, jiffies);
+	spin_lock(&shaper->lock);
+	shaper_kick(shaper);
+	spin_unlock(&shaper->lock);
 }
 
 /*
@@ -332,21 +328,6 @@
 
 
 /*
- *	Flush the shaper queues on a closedown
- */
- 
-static void shaper_flush(struct shaper *shaper)
-{
-	struct sk_buff *skb;
-
-	down(&shaper->sem);
-	while((skb=skb_dequeue(&shaper->sendq))!=NULL)
-		dev_kfree_skb(skb);
-	shaper_kick(shaper);
-	up(&shaper->sem);
-}
-
-/*
  *	Bring the interface up. We just disallow this until a 
  *	bind.
  */
@@ -375,7 +356,15 @@
 static int shaper_close(struct net_device *dev)
 {
 	struct shaper *shaper=dev->priv;
-	shaper_flush(shaper);
+	struct sk_buff *skb;
+
+	while ((skb = skb_dequeue(&shaper->sendq)) != NULL)
+		dev_kfree_skb(skb);
+
+	spin_lock_bh(&shaper->lock);
+	shaper_kick(shaper);
+	spin_unlock_bh(&shaper->lock);
+
 	del_timer_sync(&shaper->timer);
 	return 0;
 }
@@ -576,6 +565,7 @@
 	init_timer(&sh->timer);
 	sh->timer.function=shaper_timer;
 	sh->timer.data=(unsigned long)sh;
+	spin_lock_init(&sh->lock);
 }
 
 /*
include/linux/if_shaper.h +1 -1
@@ -23,7 +23,7 @@
 	__u32 shapeclock;
 	unsigned long recovery;	/* Time we can next clock a packet out on
 				   an empty queue */
-	struct semaphore sem;
+	spinlock_t lock;
         struct net_device_stats stats;
 	struct net_device *dev;
 	int  (*hard_start_xmit) (struct sk_buff *skb,
author
#6 lore raw

PATCH 5/11 SMP fix for 6pack driver

In reply to Greg KH

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

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

Drivers really only work well in SMP if they actually can be selected. This is a leftover from the time when the 6pack drive only used to be a bitrotten variant of the slip driver.

---
 drivers/net/hamradio/Kconfig |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/net/hamradio/Kconfig +1 -1
@@ -17,7 +17,7 @@
 
 config 6PACK
 	tristate "Serial port 6PACK driver"
-	depends on AX25 && BROKEN_ON_SMP
+	depends on AX25
 	---help---
 	  6pack is a transmission protocol for the data exchange between your
 	  PC and your TNC (the Terminal Node Controller acts as a kind of
#7 lore raw

Re: [05/11] SMP fix for 6pack driver

Greg KH <[email protected]> :

Quoted 8 lines

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

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

Drivers really only work well in SMP if they actually can be selected. This is a leftover from the time when the 6pack drive only used to be a bitrotten variant of the slip driver.

Is the guideline above from 28/04/2005 obsoleted ?

- It must fix a problem that causes a build error (but not for things marked CONFIG_BROKEN), an oops, a hang, data corruption, a real security issue, or some "oh, that's not good" issue. In short, something critical.

--
Ueimor

#8 lore raw

Re: [stable] Re: [05/11] SMP fix for 6pack driver

Quoted 10 lines · Francois Romieu

Greg KH <[email protected]> :

Quoted 8 lines

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

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

Drivers really only work well in SMP if they actually can be selected. This is a leftover from the time when the 6pack drive only used to be a bitrotten variant of the slip driver.

Quoted 6 lines

Is the guideline above from 28/04/2005 obsoleted ?

- It must fix a problem that causes a build error (but not for things marked CONFIG_BROKEN), an oops, a hang, data corruption, a real security issue, or some "oh, that's not good" issue. In short, something critical.

It lets the driver be built, when it previously could not be, unless the user used a config option that almost no one does...

That's pretty critical if you ask me.

thanks,

greg k-h

#9 lore raw

Re: [stable] Re: [05/11] SMP fix for 6pack driver

Quoted 18 lines · Greg KH
Quoted 10 lines · Francois Romieu

Greg KH <[email protected]> :

Quoted 8 lines

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

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

Drivers really only work well in SMP if they actually can be selected. This is a leftover from the time when the 6pack drive only used to be a bitrotten variant of the slip driver.

Quoted 6 lines

Is the guideline above from 28/04/2005 obsoleted ?

- It must fix a problem that causes a build error (but not for things marked CONFIG_BROKEN), an oops, a hang, data corruption, a real security issue, or some "oh, that's not good" issue. In short, something critical.

Quoted 4 lines

It lets the driver be built, when it previously could not be, unless the user used a config option that almost no one does...

That's pretty critical if you ask me.

I do agree with Francois regarding this issue:

AFAIR, there has been not one 2.6 kernel where this driver was available for SMP kernels. It's therefore untested which problems might arise with this driver on SMP systems. I'm not arguing against including this driver in 2.6.13, but 2.6.12.3 isn't the right place.

What surprises me most is that you accepted this patch is neither in 2.6.13-rc3 nor in 2.6.13-rc3-mm1. There seems to be either an (IMHO unfortunate) change in your policy of what patches to accept, or there's a serious problem in your patch review process.

Quoted 3 lines

thanks,

greg k-h

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#10 lore raw

Re: [stable] Re: [05/11] SMP fix for 6pack driver

Quoted 4 lines · Adrian Bunk

I do agree with Francois regarding this issue:

AFAIR, there has been not one 2.6 kernel where this driver was available for SMP kernels.

Eh... That after all is the raison d'etre for this patch :)

Quoted 3 lines

It's therefore untested which problems might arise with this driver on SMP systems. I'm not arguing against including this driver in 2.6.13, but 2.6.12.3 isn't the right place.

Nonsense. Most development activity for this stuff happens not on the internet and you won't be able to follow it unless you're a licensed ham. I've been circulating things patch since a while and nobody has been unhappy.

Quoted 4 lines

What surprises me most is that you accepted this patch is neither in 2.6.13-rc3 nor in 2.6.13-rc3-mm1. There seems to be either an (IMHO unfortunate) change in your policy of what patches to accept, or there's a serious problem in your patch review process.

I've sent it to jgarzik so it's somewhere on it's long way there.

Ralf

#11 lore raw

Re: [stable] Re: [05/11] SMP fix for 6pack driver

Quoted 9 lines · Ralf Baechle
Quoted 4 lines · Adrian Bunk

I do agree with Francois regarding this issue:

AFAIR, there has been not one 2.6 kernel where this driver was available for SMP kernels.

Eh... That after all is the raison d'etre for this patch :)

Quoted 7 lines
Quoted 3 lines

It's therefore untested which problems might arise with this driver on SMP systems. I'm not arguing against including this driver in 2.6.13, but 2.6.12.3 isn't the right place.

Nonsense. Most development activity for this stuff happens not on the internet and you won't be able to follow it unless you're a licensed ham. I've been circulating things patch since a while and nobody has been unhappy.

So why was it important to get it into 2.6.12.3?

I must have missed the announcement that the original -stable goals have been declared obsolete.

Quoted 8 lines
Quoted 4 lines

What surprises me most is that you accepted this patch is neither in 2.6.13-rc3 nor in 2.6.13-rc3-mm1. There seems to be either an (IMHO unfortunate) change in your policy of what patches to accept, or there's a serious problem in your patch review process.

I've sent it to jgarzik so it's somewhere on it's long way there.

Ralf

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
author
#12 lore raw

PATCH 6/11 tpm breaks 8139cp

In reply to Greg KH

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

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

A problem was reported that the tpm driver was interfereing with networking on the 8139 chipset. The tpm driver was using a hard coded the memory address instead of the value the BIOS was putting the chip at. This was in the tpm_lpc_bus_init function. That function can be replaced with querying the value at Vendor specific locations. This patch replaces all calls to tpm_lpc_bus_init and the hardcoding of the base address with a lookup of the address at the correct vendor location.

---
 drivers/char/tpm/tpm.c       |   76 -------------------------------------------
 drivers/char/tpm/tpm.h       |    2 -
 drivers/char/tpm/tpm_atmel.c |   16 +++++----
 drivers/char/tpm/tpm_nsc.c   |   16 +++++----
 4 files changed, 20 insertions(+), 90 deletions(-)
drivers/char/tpm/tpm_atmel.c +10 -6
@@ -22,7 +22,10 @@
 #include "tpm.h"
 
 /* Atmel definitions */
-#define	TPM_ATML_BASE			0x400
+enum tpm_atmel_addr {
+	TPM_ATMEL_BASE_ADDR_LO = 0x08,
+	TPM_ATMEL_BASE_ADDR_HI = 0x09
+};
 
 /* write status bits */
 #define	ATML_STATUS_ABORT		0x01
@@ -127,7 +130,6 @@
 	.cancel = tpm_atml_cancel,
 	.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
 	.req_complete_val = ATML_STATUS_DATA_AVAIL,
-	.base = TPM_ATML_BASE,
 	.miscdev = { .fops = &atmel_ops, },
 };
 
@@ -136,14 +138,16 @@
 {
 	u8 version[4];
 	int rc = 0;
+	int lo, hi;
 
 	if (pci_enable_device(pci_dev))
 		return -EIO;
 
-	if (tpm_lpc_bus_init(pci_dev, TPM_ATML_BASE)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	lo = tpm_read_index( TPM_ATMEL_BASE_ADDR_LO );
+	hi = tpm_read_index( TPM_ATMEL_BASE_ADDR_HI );
+
+	tpm_atmel.base = (hi<<8)|lo;
+	dev_dbg( &pci_dev->dev, "Operating with base: 0x%x\n", tpm_atmel.base);
 
 	/* verify that it is an Atmel part */
 	if (tpm_read_index(4) != 'A' || tpm_read_index(5) != 'T'
drivers/char/tpm/tpm.c +0 -76

94 lines not on this page · raw message

drivers/char/tpm/tpm.h +0 -2
@@ -79,8 +79,6 @@
 }
 
 extern void tpm_time_expired(unsigned long);
-extern int tpm_lpc_bus_init(struct pci_dev *, u16);
-
 extern int tpm_register_hardware(struct pci_dev *,
 				 struct tpm_vendor_specific *);
 extern int tpm_open(struct inode *, struct file *);
drivers/char/tpm/tpm_nsc.c +10 -6
@@ -24,6 +24,10 @@
 /* National definitions */
 #define	TPM_NSC_BASE			0x360
 #define	TPM_NSC_IRQ			0x07
+#define	TPM_NSC_BASE0_HI		0x60
+#define	TPM_NSC_BASE0_LO		0x61
+#define	TPM_NSC_BASE1_HI		0x62
+#define	TPM_NSC_BASE1_LO		0x63
 
 #define	NSC_LDN_INDEX			0x07
 #define	NSC_SID_INDEX			0x20
@@ -234,7 +238,6 @@
 	.cancel = tpm_nsc_cancel,
 	.req_complete_mask = NSC_STATUS_OBF,
 	.req_complete_val = NSC_STATUS_OBF,
-	.base = TPM_NSC_BASE,
 	.miscdev = { .fops = &nsc_ops, },
 	
 };
@@ -243,15 +246,16 @@
 				  const struct pci_device_id *pci_id)
 {
 	int rc = 0;
+	int lo, hi;
+
+	hi = tpm_read_index(TPM_NSC_BASE0_HI);
+	lo = tpm_read_index(TPM_NSC_BASE0_LO);
+
+	tpm_nsc.base = (hi<<8) | lo;
 
 	if (pci_enable_device(pci_dev))
 		return -EIO;
 
-	if (tpm_lpc_bus_init(pci_dev, TPM_NSC_BASE)) {
-		rc = -ENODEV;
-		goto out_err;
-	}
-
 	/* verify that it is a National part (SID) */
 	if (tpm_read_index(NSC_SID_INDEX) != 0xEF) {
 		rc = -ENODEV;
#13 lore raw

Re: [06/11] tpm breaks 8139cp

Quoted 6 lines · Greg KH

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

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

A problem was reported that the tpm driver was interfereing with networking on the 8139 chipset. The tpm driver was using a hard coded

ACK...this looks very similar to a patch I got from Kylene for fixing this same issue when it showed-up in FC4.

John

Signature · 2 lines
John W. Linville
[email protected]
author
#14 lore raw

PATCH 7/11 v4l cx88 hue offset fix

In reply to Greg KH

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

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

Changed hue offset to 128 to correct behavior in cx88 cards. Previously, setting 0% or 100% hue was required to avoid blue/green people on screen. Now, 50% Hue means no offset, just like bt878 stuff.

---
 drivers/media/video/cx88/cx88-video.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
drivers/media/video/cx88/cx88-video.c +1 -1
@@ -261,7 +261,7 @@
 			.default_value = 0,
 			.type          = V4L2_CTRL_TYPE_INTEGER,
 		},
-		.off                   = 0,
+		.off                   = 128,
 		.reg                   = MO_HUE,
 		.mask                  = 0x00ff,
 		.shift                 = 0,
author
#15 lore raw

PATCH 8/11 coverity: tty_ldisc_ref return null check

In reply to Greg KH

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

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

From: KAMBAROV, ZAUR <[email protected]>

We add a check of the return value of tty_ldisc_ref(), which is checked 7 out of 8 times, e.g.:

149  		ld = tty_ldisc_ref(tty);
150  		if (ld != NULL) {
151  			if (ld->set_termios)
152  				(ld->set_termios)(tty, &old_termios);
153  			tty_ldisc_deref(ld);
154  		}

This defect was found automatically by Coverity Prevent, a static analysis tool.

(akpm: presumably `ld' is never NULL. Oh well)

---
 drivers/char/tty_ioctl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/char/tty_ioctl.c +2 -2
@@ -476,11 +476,11 @@
 			ld = tty_ldisc_ref(tty);
 			switch (arg) {
 			case TCIFLUSH:
-				if (ld->flush_buffer)
+				if (ld && ld->flush_buffer)
 					ld->flush_buffer(tty);
 				break;
 			case TCIOFLUSH:
-				if (ld->flush_buffer)
+				if (ld && ld->flush_buffer)
 					ld->flush_buffer(tty);
 				/* fall through */
 			case TCOFLUSH:
author
#16 lore raw

PATCH 9/11 uml: fix TT mode by reverting "use fork instead of clone"

In reply to Greg KH

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

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

From: Jeff Dike <[email protected]>, Paolo 'Blaisorblade' Giarrusso <[email protected]>

Revert the following patch, because of miscompilation problems in different environments leading to UML not working *at all* in TT mode; it was merged lately in 2.6 development cycle, a little after being written, and has caused problems to lots of people; I know it's a bit too long, but it shouldn't have been merged in first place, so I still apply for inclusion in the -stable tree. Anyone using this feature currently is either using some older kernel (some reports even used 2.6.12-rc4-mm2) or using this patch, as included in my -bs patchset.

For now there's not yet a fix for this patch, so for now the best thing is to drop it (which was widely reported to give a working kernel).

"Convert the boot-time host ptrace testing from clone to fork. They were essentially doing fork anyway. This cleans up the code a bit, and makes valgrind a bit happier about grinding it."

URL:
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=98fdffccea6cc3fe9dba32c0fcc310bcb5d71529

---
 arch/um/kernel/process.c |   48 ++++++++++++++++++++++++++++-------------------
 1 files changed, 29 insertions(+), 19 deletions(-)
arch/um/kernel/process.c +29 -19
@@ -130,7 +130,7 @@
 	return(arg.pid);
 }
 
-static int ptrace_child(void)
+static int ptrace_child(void *arg)
 {
 	int ret;
 	int pid = os_getpid(), ppid = getppid();
@@ -159,16 +159,20 @@
 	_exit(ret);
 }
 
-static int start_ptraced_child(void)
+static int start_ptraced_child(void **stack_out)
 {
+	void *stack;
+	unsigned long sp;
 	int pid, n, status;
 	
-	pid = fork();
-	if(pid == 0)
-		ptrace_child();
-
+	stack = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC,
+		     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if(stack == MAP_FAILED)
+		panic("check_ptrace : mmap failed, errno = %d", errno);
+	sp = (unsigned long) stack + PAGE_SIZE - sizeof(void *);
+	pid = clone(ptrace_child, (void *) sp, SIGCHLD, NULL);
 	if(pid < 0)
-		panic("check_ptrace : fork failed, errno = %d", errno);
+		panic("check_ptrace : clone failed, errno = %d", errno);
 	CATCH_EINTR(n = waitpid(pid, &status, WUNTRACED));
 	if(n < 0)
 		panic("check_ptrace : wait failed, errno = %d", errno);
@@ -176,6 +180,7 @@
 		panic("check_ptrace : expected SIGSTOP, got status = %d",
 		      status);
 
+	*stack_out = stack;
 	return(pid);
 }
 
@@ -183,12 +188,12 @@
  * just avoid using sysemu, not panic, but only if SYSEMU features are broken.
  * So only for SYSEMU features we test mustpanic, while normal host features
  * must work anyway!*/
-static int stop_ptraced_child(int pid, int exitcode, int mustexit)
+static int stop_ptraced_child(int pid, void *stack, int exitcode, int mustpanic)
 {
 	int status, n, ret = 0;
 
 	if(ptrace(PTRACE_CONT, pid, 0, 0) < 0)
-		panic("stop_ptraced_child : ptrace failed, errno = %d", errno);
+		panic("check_ptrace : ptrace failed, errno = %d", errno);
 	CATCH_EINTR(n = waitpid(pid, &status, 0));
 	if(!WIFEXITED(status) || (WEXITSTATUS(status) != exitcode)) {
 		int exit_with = WEXITSTATUS(status);
@@ -199,13 +204,15 @@
 		printk("check_ptrace : child exited with exitcode %d, while "
 		      "expecting %d; status 0x%x", exit_with,
 		      exitcode, status);
-		if (mustexit)
+		if (mustpanic)
 			panic("\n");
 		else
 			printk("\n");
 		ret = -1;
 	}
 
+	if(munmap(stack, PAGE_SIZE) < 0)
+		panic("check_ptrace : munmap failed, errno = %d", errno);
 	return ret;
 }
 
@@ -227,11 +234,12 @@
 
 static void __init check_sysemu(void)
 {
+	void *stack;
 	int pid, syscall, n, status, count=0;
 
 	printk("Checking syscall emulation patch for ptrace...");
 	sysemu_supported = 0;
-	pid = start_ptraced_child();
+	pid = start_ptraced_child(&stack);
 
 	if(ptrace(PTRACE_SYSEMU, pid, 0, 0) < 0)
 		goto fail;
@@ -249,7 +257,7 @@
 		panic("check_sysemu : failed to modify system "
 		      "call return, errno = %d", errno);
 
-	if (stop_ptraced_child(pid, 0, 0) < 0)
+	if (stop_ptraced_child(pid, stack, 0, 0) < 0)
 		goto fail_stopped;
 
 	sysemu_supported = 1;
@@ -257,7 +265,7 @@
 	set_using_sysemu(!force_sysemu_disabled);
 
 	printk("Checking advanced syscall emulation patch for ptrace...");
-	pid = start_ptraced_child();
+	pid = start_ptraced_child(&stack);
 	while(1){
 		count++;
 		if(ptrace(PTRACE_SYSEMU_SINGLESTEP, pid, 0, 0) < 0)
@@ -282,7 +290,7 @@
 			break;
 		}
 	}
-	if (stop_ptraced_child(pid, 0, 0) < 0)
+	if (stop_ptraced_child(pid, stack, 0, 0) < 0)
 		goto fail_stopped;
 
 	sysemu_supported = 2;
@@ -293,17 +301,18 @@
 	return;
 
 fail:
-	stop_ptraced_child(pid, 1, 0);
+	stop_ptraced_child(pid, stack, 1, 0);
 fail_stopped:
 	printk("missing\n");
 }
 
 void __init check_ptrace(void)
 {
+	void *stack;
 	int pid, syscall, n, status;
 
 	printk("Checking that ptrace can change system call numbers...");
-	pid = start_ptraced_child();
+	pid = start_ptraced_child(&stack);
 
 	if (ptrace(PTRACE_OLDSETOPTIONS, pid, 0, (void *)PTRACE_O_TRACESYSGOOD) < 0)
 		panic("check_ptrace: PTRACE_SETOPTIONS failed, errno = %d", errno);
@@ -330,7 +339,7 @@
 			break;
 		}
 	}
-	stop_ptraced_child(pid, 0, 1);
+	stop_ptraced_child(pid, stack, 0, 1);
 	printk("OK\n");
 	check_sysemu();
 }
@@ -362,10 +371,11 @@
 static inline int check_skas3_ptrace_support(void)
 {
 	struct ptrace_faultinfo fi;
+	void *stack;
 	int pid, n, ret = 1;
 
 	printf("Checking for the skas3 patch in the host...");
-	pid = start_ptraced_child();
+	pid = start_ptraced_child(&stack);
 
 	n = ptrace(PTRACE_FAULTINFO, pid, 0, &fi);
 	if (n < 0) {
@@ -380,7 +390,7 @@
 	}
 
 	init_registers(pid);
-	stop_ptraced_child(pid, 1, 1);
+	stop_ptraced_child(pid, stack, 1, 1);
 
 	return(ret);
 }

-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual core and dual graphics technology at this free one hour event hosted by HP, AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

author
#17 lore raw

PATCH 10/11 fix semaphore handling in __unregister_chrdev_region

In reply to Greg KH

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

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

This up() should be down() instead.

---
 fs/char_dev.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
fs/char_dev.c +1 -1
@@ -139,7 +139,7 @@
 	struct char_device_struct *cd = NULL, **cp;
 	int i = major_to_index(major);
 
-	up(&chrdevs_lock);
+	down(&chrdevs_lock);
 	for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
 		if ((*cp)->major == major &&
 		    (*cp)->baseminor == baseminor &&
author
#18 lore raw

PATCH 11/11 x86_64: TASK_SIZE fixes for compatibility mode processes

In reply to Greg KH

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

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

From: Suresh Siddha <[email protected]>

Appended patch will setup compatibility mode TASK_SIZE properly. This will fix atleast three known bugs that can be encountered while running compatibility mode apps.

a) A malicious 32bit app can have an elf section at 0xffffe000. During exec of this app, we will have a memory leak as insert_vm_struct() is not checking for return value in syscall32_setup_pages() and thus not freeing the vma allocated for the vsyscall page. And instead of exec failing (as it has addresses > TASK_SIZE), we were allowing it to succeed previously.

b) With a 32bit app, hugetlb_get_unmapped_area/arch_get_unmapped_area may return addresses beyond 32bits, ultimately causing corruption because of wrap-around and resulting in SEGFAULT, instead of returning ENOMEM.

c) 32bit app doing this below mmap will now fail.

mmap((void *)(0xFFFFE000UL), 0x10000UL, PROT_READ|PROT_WRITE,

	MAP_FIXED|MAP_PRIVATE|MAP_ANON, 0, 0);
---
 arch/x86_64/ia32/ia32_binfmt.c  |    5 +----
 arch/x86_64/kernel/process.c    |    4 ++--
 arch/x86_64/kernel/ptrace.c     |   17 ++++++++++-------
 arch/x86_64/kernel/sys_x86_64.c |   14 ++++----------
 arch/x86_64/mm/fault.c          |    2 +-
 include/asm-x86_64/a.out.h      |    2 +-
 include/asm-x86_64/processor.h  |   11 ++++++-----
 7 files changed, 25 insertions(+), 30 deletions(-)
arch/x86_64/ia32/ia32_binfmt.c +1 -4
@@ -46,7 +46,7 @@
 
 #define IA32_EMULATOR 1
 
-#define ELF_ET_DYN_BASE		(TASK_UNMAPPED_32 + 0x1000000)
+#define ELF_ET_DYN_BASE		(TASK_UNMAPPED_BASE + 0x1000000)
 
 #undef ELF_ARCH
 #define ELF_ARCH EM_386
@@ -307,9 +307,6 @@
 
 #define elf_addr_t __u32
 
-#undef TASK_SIZE
-#define TASK_SIZE 0xffffffff
-
 static void elf32_init(struct pt_regs *);
 
 #define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1
arch/x86_64/kernel/process.c +2 -2
@@ -656,7 +656,7 @@
 
 	switch (code) { 
 	case ARCH_SET_GS:
-		if (addr >= TASK_SIZE) 
+		if (addr >= TASK_SIZE_OF(task))
 			return -EPERM; 
 		cpu = get_cpu();
 		/* handle small bases via the GDT because that's faster to 
@@ -682,7 +682,7 @@
 	case ARCH_SET_FS:
 		/* Not strictly needed for fs, but do it for symmetry
 		   with gs */
-		if (addr >= TASK_SIZE)
+		if (addr >= TASK_SIZE_OF(task))
 			return -EPERM; 
 		cpu = get_cpu();
 		/* handle small bases via the GDT because that's faster to 
arch/x86_64/kernel/ptrace.c +10 -7
@@ -257,12 +257,12 @@
 			value &= 0xffff;
 			return 0;
 		case offsetof(struct user_regs_struct,fs_base):
-			if (value >= TASK_SIZE)
+			if (value >= TASK_SIZE_OF(child))
 				return -EIO;
 			child->thread.fs = value;
 			return 0;
 		case offsetof(struct user_regs_struct,gs_base):
-			if (value >= TASK_SIZE)
+			if (value >= TASK_SIZE_OF(child))
 				return -EIO;
 			child->thread.gs = value;
 			return 0;
@@ -279,7 +279,7 @@
 			break;
 		case offsetof(struct user_regs_struct, rip):
 			/* Check if the new RIP address is canonical */
-			if (value >= TASK_SIZE)
+			if (value >= TASK_SIZE_OF(child))
 				return -EIO;
 			break;
 	}
@@ -419,6 +419,8 @@
 		break;
 
 	case PTRACE_POKEUSR: /* write the word at location addr in the USER area */
+	{
+		int dsize = test_tsk_thread_flag(child, TIF_IA32) ? 3 : 7;
 		ret = -EIO;
 		if ((addr & 7) ||
 		    addr > sizeof(struct user) - 7)
@@ -430,22 +432,22 @@
 			break;
 		/* Disallows to set a breakpoint into the vsyscall */
 		case offsetof(struct user, u_debugreg[0]):
-			if (data >= TASK_SIZE-7) break;
+			if (data >= TASK_SIZE_OF(child) - dsize) break;
 			child->thread.debugreg0 = data;
 			ret = 0;
 			break;
 		case offsetof(struct user, u_debugreg[1]):
-			if (data >= TASK_SIZE-7) break;
+			if (data >= TASK_SIZE_OF(child) - dsize) break;
 			child->thread.debugreg1 = data;
 			ret = 0;
 			break;
 		case offsetof(struct user, u_debugreg[2]):
-			if (data >= TASK_SIZE-7) break;
+			if (data >= TASK_SIZE_OF(child) - dsize) break;
 			child->thread.debugreg2 = data;
 			ret = 0;
 			break;
 		case offsetof(struct user, u_debugreg[3]):
-			if (data >= TASK_SIZE-7) break;
+			if (data >= TASK_SIZE_OF(child) - dsize) break;
 			child->thread.debugreg3 = data;
 			ret = 0;
 			break;
@@ -469,6 +471,7 @@
 		  break;
 		}
 		break;
+	}
 	case PTRACE_SYSCALL: /* continue and stop at next (return from) syscall */
 	case PTRACE_CONT:    /* restart after signal. */
 
arch/x86_64/kernel/sys_x86_64.c +4 -10
@@ -68,13 +68,7 @@
 static void find_start_end(unsigned long flags, unsigned long *begin,
 			   unsigned long *end)
 {
-#ifdef CONFIG_IA32_EMULATION
-	if (test_thread_flag(TIF_IA32)) { 
-		*begin = TASK_UNMAPPED_32;
-		*end = IA32_PAGE_OFFSET; 
-	} else 
-#endif
-	if (flags & MAP_32BIT) { 
+	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT)) {
 		/* This is usually used needed to map code in small
 		   model, so it needs to be in the first 31bit. Limit
 		   it to that.  This means we need to move the
@@ -84,10 +78,10 @@
 		   of playground for now. -AK */ 
 		*begin = 0x40000000; 
 		*end = 0x80000000;		
-	} else { 
-		*begin = TASK_UNMAPPED_64; 
+	} else {
+		*begin = TASK_UNMAPPED_BASE;
 		*end = TASK_SIZE; 
-		}
+	}
 } 
 
 unsigned long
arch/x86_64/mm/fault.c +1 -1
@@ -350,7 +350,7 @@
 	 * (error_code & 4) == 0, and that the fault was not a
 	 * protection error (error_code & 1) == 0.
 	 */
-	if (unlikely(address >= TASK_SIZE)) {
+	if (unlikely(address >= TASK_SIZE64)) {
 		if (!(error_code & 5) &&
 		      ((address >= VMALLOC_START && address < VMALLOC_END) ||
 		       (address >= MODULES_VADDR && address < MODULES_END))) {
include/asm-x86_64/a.out.h +1 -1
@@ -21,7 +21,7 @@
 
 #ifdef __KERNEL__
 #include <linux/thread_info.h>
-#define STACK_TOP (test_thread_flag(TIF_IA32) ? IA32_PAGE_OFFSET : TASK_SIZE)
+#define STACK_TOP TASK_SIZE
 #endif
 
 #endif /* __A_OUT_GNU_H__ */
include/asm-x86_64/processor.h +6 -5
@@ -160,16 +160,17 @@
 /*
  * User space process size. 47bits minus one guard page.
  */
-#define TASK_SIZE	(0x800000000000UL - 4096)
+#define TASK_SIZE64	(0x800000000000UL - 4096)
 
 /* This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
 #define IA32_PAGE_OFFSET ((current->personality & ADDR_LIMIT_3GB) ? 0xc0000000 : 0xFFFFe000)
-#define TASK_UNMAPPED_32 PAGE_ALIGN(IA32_PAGE_OFFSET/3)
-#define TASK_UNMAPPED_64 PAGE_ALIGN(TASK_SIZE/3) 
-#define TASK_UNMAPPED_BASE	\
-	(test_thread_flag(TIF_IA32) ? TASK_UNMAPPED_32 : TASK_UNMAPPED_64)  
+
+#define TASK_SIZE 		(test_thread_flag(TIF_IA32) ? IA32_PAGE_OFFSET : TASK_SIZE64)
+#define TASK_SIZE_OF(child) 	((test_tsk_thread_flag(child, TIF_IA32)) ? IA32_PAGE_OFFSET : TASK_SIZE64)
+
+#define TASK_UNMAPPED_BASE	PAGE_ALIGN(TASK_SIZE/3)
 
 /*
  * Size of io_bitmap.
#19 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 1 line · Greg KH

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

I think the patch is too risky for stable. I had even my doubts for mainline.

-Andi

#20 lore raw

Re: [stable] Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 5 lines · Andi Kleen
Quoted 1 line · Greg KH

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

I think the patch is too risky for stable. I had even my doubts for mainline.

Ok, thanks, I've removed it out of the queue.

greg k-h

#21 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

In reply to Andi Kleen
Quoted 5 lines · Andi Kleen
Quoted 1 line · Greg KH

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

I think the patch is too risky for stable. I had even my doubts for mainline.

hmm.. Main reason why Andrew posted this for stable series is because of the memory leak issue mentioned in the patch changeset comments...

We have not seen any stability issues because of this patch so far(its been there for more than a month in -mm series). Lack of this patch is actually causing us more troubles (DOS/app failures/..).

thanks,
suresh

#22 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 10 lines · Suresh B
Quoted 5 lines · Andi Kleen
Quoted 1 line · Greg KH

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

I think the patch is too risky for stable. I had even my doubts for mainline.

hmm.. Main reason why Andrew posted this for stable series is because of the memory leak issue mentioned in the patch changeset comments...

I would say if Andi has concerns for this stable series, we should indeed leave it out. That said, I will be testing this patch a bit further myself, and because it does address a real memory leak issue, we should consider it or another fix for stable 2.6.12.4.

Justin M. Forbes

#23 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 1 line · Justin M. Forbes

That said, I will be testing this patch a bit further

Thanks. Let me know if you see any issues.

Quoted 2 lines

myself, and because it does address a real memory leak issue, we should consider it or another fix for stable 2.6.12.4.

Appended patch will just fix the memory leak issue. Atleast, we should apply this.

thanks,
suresh
--

malicious 32bit app can have an elf section at 0xffffe000. During exec of this app, we will have a memory leak as insert_vm_struct() is not checking for return value in syscall32_setup_pages() and thus not freeing the vma allocated for the vsyscall page.

Check the return value and free the vma incase of failure.

arch/x86_64/ia32/syscall32.c renamed from arch/x86_64/ia32/syscall32.c.orig +6 -1
@@ -57,6 +57,7 @@
 	int npages = (VSYSCALL32_END - VSYSCALL32_BASE) >> PAGE_SHIFT;
 	struct vm_area_struct *vma;
 	struct mm_struct *mm = current->mm;
+	int ret;
 
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
 	if (!vma)
@@ -78,7 +79,11 @@
 	vma->vm_mm = mm;
 
 	down_write(&mm->mmap_sem);
-	insert_vm_struct(mm, vma);
+	if ((ret = insert_vm_struct(mm, vma))) {
+		up_write(&mm->mmap_sem);
+		kmem_cache_free(vm_area_cachep, vma);
+		return ret;
+	}
 	mm->total_vm += npages;
 	up_write(&mm->mmap_sem);
 	return 0;
#24 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 10 lines · Suresh B
Quoted 1 line · Justin M. Forbes

That said, I will be testing this patch a bit further

Thanks. Let me know if you see any issues.

Quoted 2 lines

myself, and because it does address a real memory leak issue, we should consider it or another fix for stable 2.6.12.4.

Appended patch will just fix the memory leak issue. Atleast, we should apply this.

Looks good. Thanks, Suresh.

-Andi

#25 lore raw

Re: [stable] Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

Quoted 6 lines · * Andi Kleen ([email protected]) wrote:
Quoted 5 lines · Suresh B
Quoted 1 line · Justin M. Forbes

That said, I will be testing this patch a bit further

Thanks. Let me know if you see any issues.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

myself, and because it does address a real memory leak issue, we should consider it or another fix for stable 2.6.12.4.

Appended patch will just fix the memory leak issue. Atleast, we should apply this.

Looks good. Thanks, Suresh.

Queued for next -stable.

thanks,
-chris

#26 lore raw

Re: [11/11] x86_64: TASK_SIZE fixes for compatibility mode processes

In reply to Greg KH
Quoted 4 lines · Greg KH

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

Huh? Cc: in here?

Quoted 4 lines

---

Signature · 4 lines
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513