Re: [stable] [patch 00/48] 2.6.27.32-stable review

Re: [stable] [patch 00/48] 2.6.27.32-stable review

[Greg KH]

On Sat, Sep 12, 2009 at 12:22:22AM +0200, Tilman Schmidt wrote:
> [On holiday, answering from my nominally internet capable cellphone - please make allowances ...]
> 
> The stable patch claiming to correspond to Linus' tree patch
> 56f7efe48d57dda9e59e23ab161c118271cce815 and merged in 2.6.27.sth
> (eyact version not at hand) and 2.6.28.10 differs from the backported
> patch I submitted for stable myself. It breaks the driver completely,
> causing an oops and system malfunctions as soon as the device is
> connected. (Sorry for not noticing the vital difference at the time.)

No problem.

> The attached patch would fix the error by moving the code inswrted at
> the wrong place by the broken backported patch,, to the correct place
> where it went in the original patch. 

I see no patch attached here :(

> Alternativelyy, the broken backported could be reverted and my
> original bacport merged instead, if you can still find it.

I can not.

> Sorry for lack of references as i don't have acces to my development
> machine from here. Hope to have Cleared up the situation, anyway.

Can you send me what you need applied to the latest 2.6.27-stable tree
to fix this up so we can finally resolve this?

thanks,

greg k-h

Re: [Stable-review] [stable] [patch 00/48] 2.6.27.32-stable review

[Stefan Bader]

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

Greg KH wrote:
> On Sat, Sep 12, 2009 at 12:22:22AM +0200, Tilman Schmidt wrote:
>> [On holiday, answering from my nominally internet capable cellphone - please make allowances ...]
>>
>> The stable patch claiming to correspond to Linus' tree patch
>> 56f7efe48d57dda9e59e23ab161c118271cce815 and merged in 2.6.27.sth
>> (eyact version not at hand) and 2.6.28.10 differs from the backported
>> patch I submitted for stable myself. It breaks the driver completely,
>> causing an oops and system malfunctions as soon as the device is
>> connected. (Sorry for not noticing the vital difference at the time.)
> 
> No problem.
> 
>> The attached patch would fix the error by moving the code inswrted at
>> the wrong place by the broken backported patch,, to the correct place
>> where it went in the original patch. 
> 
> I see no patch attached here :(
> 
>> Alternativelyy, the broken backported could be reverted and my
>> original bacport merged instead, if you can still find it.
> 
> I can not.
> 
>> Sorry for lack of references as i don't have acces to my development
>> machine from here. Hope to have Cleared up the situation, anyway.
> 
> Can you send me what you need applied to the latest 2.6.27-stable tree
> to fix this up so we can finally resolve this?
> 

I believe it was applicable to 2.6.27.y without problems. This is the patch I 
sent to Tilman and stable. (Sorry, I am currently at the airport and do not 
have 2.6.27.y with me)

-Stefan

> thanks,
> 
> greg k-h
> 
> _______________________________________________
> Stable-review mailing list
> Stable-review@linux.kernel.org
> http://linux.kernel.org/mailman/listinfo/stable-review


[-- Attachment #2: 0001-UBUNTU-SAUCE-Fix-incorrect-stable-backport-to-bas_.patch --]
[-- Type: text/x-diff, Size: 1950 bytes --]

>From 55559d69f6a84ae295e8a1b0cbabc7d6c7e5f18a Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Tue, 25 Aug 2009 17:35:56 +0200
Subject: [PATCH] Fix incorrect stable backport to bas_gigaset

BugLink: http://bugs.launchpad.net/bugs/417732

commit 56f7efe48d57dda9e59e23ab161c118271cce815
Author: Tilman Schmidt <tilman@imap.cc>
Date:   Wed Apr 15 03:25:43 2009 -0700

    bas_gigaset: correctly allocate USB interrupt transfer buffer

    [ Upstream commit 170ebf85160dd128e1c4206cc197cce7d1424705 ]

This incorrect backport to 2.6.28.10 placed some code into the probe function
which used a pointer before it was initialized. Moving this to the correct
place (as it is in upstream).

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Acked-by: Steve Conklin <steve.conklin@canonical.com>
---
 drivers/isdn/gigaset/bas-gigaset.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index fcec2df..3990eae 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -2140,8 +2140,16 @@ static int gigaset_initcshw(struct cardstate *cs)
 	struct bas_cardstate *ucs;
 
 	cs->hw.bas = ucs = kmalloc(sizeof *ucs, GFP_KERNEL);
-	if (!ucs)
+	if (!ucs) {
+		pr_err("out of memory\n");
+		return 0;
+	}
+	ucs->int_in_buf = kmalloc(IP_MSGSIZE, GFP_KERNEL);
+	if (!ucs->int_in_buf) {
+		kfree(ucs);
+		pr_err("out of memory\n");
 		return 0;
+	}
 
 	ucs->urb_cmd_in = NULL;
 	ucs->urb_cmd_out = NULL;
@@ -2236,12 +2244,6 @@ static int gigaset_probe(struct usb_interface *interface,
 		}
 		hostif = interface->cur_altsetting;
 	}
-	ucs->int_in_buf = kmalloc(IP_MSGSIZE, GFP_KERNEL);
-	if (!ucs->int_in_buf) {
-		kfree(ucs);
-		pr_err("out of memory\n");
-		return 0;
-	}
 
 	/* Reject application specific interfaces
 	 */
-- 
1.6.0.4

Re: [stable] [patch 00/48] 2.6.27.32-stable review

[Tilman Schmidt]

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

Am 16.09.2009 23:09 schrieb Greg KH:
> Can you send me what you need applied to the latest 2.6.27-stable tree
> to fix this up so we can finally resolve this?

The patch that should have been applied is here:

http://patchwork.kernel.org/patch/13845/

I'm including it below for completeness.

The patch that was applied to 2.6.27-stable instead is this:

http://patchwork.kernel.org/patch/20855/
(commit 221418f9a25cd96aff99d6aba41c177f6eb00853)

The difference is in blob 5 which inserts the new code at the wrong place.
I have no idea who or what caused that change.

So commit 221418f9a25cd96aff99d6aba41c177f6eb00853 should be reverted and
my original patch below applied instead. The patch by Stefan Bader is
equivalent to that but additionally backports two pr_err() calls from
mainline.

Thanks,
Tilman



-------- Original-Nachricht --------
Betreff: [PATCH 2.6.27/28-stable] bas_gigaset: correctly allocate USB interrupt transfer buffer
Datum: Mon, 23 Mar 2009 22:47:06 +0100 (CET)
Von: Tilman Schmidt <tilman@imap.cc>
An: stable@kernel.org
CC: Kolja Waschk <kawk@users.sourceforge.net>, Hansjoerg Lipp <hjlipp@web.de>, davem@davemloft.net, linux-kernel@vger.kernel.org,        netdev@vger.kernel.org

Greg,

the following patch backports the Gigaset driver bugfix merged for
2.6.29 as commit 170ebf85160dd128e1c4206cc197cce7d1424705 to both
v2.6.27.x and v2.6.28.x. I would appreciate to see it included in
your stable series.

Thanks,
Tilman

bas_gigaset: correctly allocate USB interrupt transfer buffer

commit 170ebf85160dd128e1c4206cc197cce7d1424705 upstream.

Every USB transfer buffer has to be allocated individually by kmalloc.

Impact: bugfix, no functional change

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Tested-by: Kolja Waschk <kawk@users.sourceforge.net>
---
 drivers/isdn/gigaset/bas-gigaset.c |   15 +++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

--- a/drivers/isdn/gigaset/bas-gigaset.c	2008-12-25 00:26:37.000000000 +0100
+++ b/drivers/isdn/gigaset/bas-gigaset.c	2009-03-17 23:38:30.000000000 +0100
@@ -46,6 +46,9 @@ MODULE_PARM_DESC(cidmode, "Call-ID mode"
 /* length limit according to Siemens 3070usb-protokoll.doc ch. 2.1 */
 #define IF_WRITEBUF 264
 
+/* interrupt pipe message size according to ibid. ch. 2.2 */
+#define IP_MSGSIZE 3
+
 /* Values for the Gigaset 307x */
 #define USB_GIGA_VENDOR_ID      0x0681
 #define USB_3070_PRODUCT_ID     0x0001
@@ -110,7 +113,7 @@ struct bas_cardstate {
 	unsigned char		*rcvbuf;	/* AT reply receive buffer */
 
 	struct urb		*urb_int_in;	/* URB for interrupt pipe */
-	unsigned char		int_in_buf[3];
+	unsigned char		*int_in_buf;
 
 	spinlock_t		lock;		/* locks all following */
 	int			basstate;	/* bitmap (BS_*) */
@@ -657,7 +660,7 @@ static void read_int_callback(struct urb
 	}
 
 	/* drop incomplete packets even if the missing bytes wouldn't matter */
-	if (unlikely(urb->actual_length < 3)) {
+	if (unlikely(urb->actual_length < IP_MSGSIZE)) {
 		dev_warn(cs->dev, "incomplete interrupt packet (%d bytes)\n",
 			 urb->actual_length);
 		goto resubmit;
@@ -2127,6 +2130,7 @@ static void gigaset_reinitbcshw(struct b
 static void gigaset_freecshw(struct cardstate *cs)
 {
 	/* timers, URBs and rcvbuf are disposed of in disconnect */
+	kfree(cs->hw.bas->int_in_buf);
 	kfree(cs->hw.bas);
 	cs->hw.bas = NULL;
 }
@@ -2138,6 +2142,11 @@ static int gigaset_initcshw(struct cards
 	cs->hw.bas = ucs = kmalloc(sizeof *ucs, GFP_KERNEL);
 	if (!ucs)
 		return 0;
+	ucs->int_in_buf = kmalloc(IP_MSGSIZE, GFP_KERNEL);
+	if (!ucs->int_in_buf) {
+		kfree(ucs);
+		return 0;
+	}
 
 	ucs->urb_cmd_in = NULL;
 	ucs->urb_cmd_out = NULL;
@@ -2290,7 +2299,7 @@ static int gigaset_probe(struct usb_inte
 	usb_fill_int_urb(ucs->urb_int_in, udev,
 			 usb_rcvintpipe(udev,
 					(endpoint->bEndpointAddress) & 0x0f),
-			 ucs->int_in_buf, 3, read_int_callback, cs,
+			 ucs->int_in_buf, IP_MSGSIZE, read_int_callback, cs,
 			 endpoint->bInterval);
 	if ((rc = usb_submit_urb(ucs->urb_int_in, GFP_KERNEL)) != 0) {
 		dev_err(cs->dev, "could not submit interrupt URB: %s\n",



-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [stable] [patch 00/48] 2.6.27.32-stable review

[Greg KH]

On Fri, Sep 18, 2009 at 03:15:15PM +0200, Tilman Schmidt wrote:
> Am 16.09.2009 23:09 schrieb Greg KH:
> > Can you send me what you need applied to the latest 2.6.27-stable tree
> > to fix this up so we can finally resolve this?
> 
> The patch that should have been applied is here:
> 
> http://patchwork.kernel.org/patch/13845/
> 
> I'm including it below for completeness.
> 
> The patch that was applied to 2.6.27-stable instead is this:
> 
> http://patchwork.kernel.org/patch/20855/
> (commit 221418f9a25cd96aff99d6aba41c177f6eb00853)
> 
> The difference is in blob 5 which inserts the new code at the wrong place.
> I have no idea who or what caused that change.
> 
> So commit 221418f9a25cd96aff99d6aba41c177f6eb00853 should be reverted and
> my original patch below applied instead. The patch by Stefan Bader is
> equivalent to that but additionally backports two pr_err() calls from
> mainline.

I took Stefan's patch as it was simpler.  Let me know if the problem is
still present.

thanks,

greg k-h

Re: [stable] [patch 00/48] 2.6.27.32-stable review

[Tilman Schmidt]

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

Am 01.10.2009 17:30 schrieb Greg KH:
> On Fri, Sep 18, 2009 at 03:15:15PM +0200, Tilman Schmidt wrote:
>>
>> So commit 221418f9a25cd96aff99d6aba41c177f6eb00853 should be reverted and
>> my original patch below applied instead. The patch by Stefan Bader is
>> equivalent to that but additionally backports two pr_err() calls from
>> mainline.
> 
> I took Stefan's patch as it was simpler.  Let me know if the problem is
> still present.

Stefan's patch is fine. I just did a quick test of
fix-incorrect-stable-backport-to-bas_gigaset.patch
on top of 2.6.27.35 and the problem is gone.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [stable] [patch 00/48] 2.6.27.32-stable review

[Greg KH]

On Sat, Oct 03, 2009 at 12:36:54AM +0200, Tilman Schmidt wrote:
> Am 01.10.2009 17:30 schrieb Greg KH:
> > On Fri, Sep 18, 2009 at 03:15:15PM +0200, Tilman Schmidt wrote:
> >>
> >> So commit 221418f9a25cd96aff99d6aba41c177f6eb00853 should be reverted and
> >> my original patch below applied instead. The patch by Stefan Bader is
> >> equivalent to that but additionally backports two pr_err() calls from
> >> mainline.
> > 
> > I took Stefan's patch as it was simpler.  Let me know if the problem is
> > still present.
> 
> Stefan's patch is fine. I just did a quick test of
> fix-incorrect-stable-backport-to-bas_gigaset.patch
> on top of 2.6.27.35 and the problem is gone.

Great, thanks for testing and letting me know.

greg k-h