* Re: [PATCH 7/9] can: janz-ican3: cleanup of ican3_to_can_frame and can_frame_to_ican3
From: David Miller @ 2010-12-26 3:32 UTC (permalink / raw)
To: mkl-bIcnvbaLZ9MEGnE8C9+IrQ
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, iws-lulEs6mt1IksTUYHLfqkUA
In-Reply-To: <1293288034-22428-8-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
From: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Date: Sat, 25 Dec 2010 15:40:32 +0100
> @@ -1421,6 +1421,9 @@ static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev)
> void __iomem *desc_addr;
> unsigned long flags;
>
> + if (can_dropped_invalid_skb(dev, skb))
> + return NETDEV_TX_OK;
> +
You never compile tested this.
Merry Christmas.
^ permalink raw reply
* Re: [net-next-2.6 00/15][pull-request] Intel Wired LAN Driver Update
From: David Miller @ 2010-12-26 3:26 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1293257174-15498-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: jeffrey.t.kirsher@intel.com
Date: Fri, 24 Dec 2010 22:05:59 -0800
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> The following series addresses the more significant issues reported by
> checkpatch when run against the e1000e driver files. The less serious
> whitespace and lines over 80 characters issues are not addressed. In
> addition, added support for anti-spoofing and X540 SR-IOV.
>
> The remaining warnings for msleep < 20ms will be addressed in a follow-on
> patch after a thorough investigation into the timings tolerated by the
> hardware.
>
> The following changes since commit e1928c86c4829703b800c81cc9edc939b5634e6f:
>
> cnic: Add FCoE support on 57712
>
> are available in the git repository at:
>
> master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6.git master
Pulled, thanks Jeff.
^ permalink raw reply
* RE: [PATCH 00/10] bna: Update Brocade BNA Ethernet driver to v2.3.2.3
From: Rasesh Mody @ 2010-12-26 3:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20101225.191813.193696521.davem@davemloft.net>
Thanks David!
>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Saturday, December 25, 2010 7:18 PM
>To: Rasesh Mody
>Cc: netdev@vger.kernel.org
>Subject: Re: [PATCH 00/10] bna: Update Brocade BNA Ethernet driver to
>v2.3.2.3
>
>From: Rasesh Mody <rmody@brocade.com>
>Date: Tue, 21 Dec 2010 23:23:06 -0800
>
>> The following patch set updates the Brocade BNA driver to v2.3.2.3.
>>
>> The patches are generated, compiled and tested against net-next-2.6
>(2.6.37-rc1).
>>
>> Rasesh Mody (10):
>> bna: TxRx and datapath fix
>> bna: Port enable disable sync and txq priority fix
>> bna: Fix ethtool register dump and reordered an API
>> bna: Enable pure priority tagged packet reception and rxf uninit
>> cleanup fix
>> bna: Fix for TX queue
>> bna: IOC uninit check and misc cleanup
>> bna: Removed unused code
>> bna: Restore VLAN filter table
>> bna: IOC failure auto recovery fix
>> bna: Update the driver version to 2.3.2.3
>
>All applied, thanks.
^ permalink raw reply
* Re: [PATCH 00/10] bna: Update Brocade BNA Ethernet driver to v2.3.2.3
From: David Miller @ 2010-12-26 3:18 UTC (permalink / raw)
To: rmody; +Cc: netdev
In-Reply-To: <1293002596-3239-1-git-send-email-rmody@brocade.com>
From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 21 Dec 2010 23:23:06 -0800
> The following patch set updates the Brocade BNA driver to v2.3.2.3.
>
> The patches are generated, compiled and tested against net-next-2.6 (2.6.37-rc1).
>
> Rasesh Mody (10):
> bna: TxRx and datapath fix
> bna: Port enable disable sync and txq priority fix
> bna: Fix ethtool register dump and reordered an API
> bna: Enable pure priority tagged packet reception and rxf uninit
> cleanup fix
> bna: Fix for TX queue
> bna: IOC uninit check and misc cleanup
> bna: Removed unused code
> bna: Restore VLAN filter table
> bna: IOC failure auto recovery fix
> bna: Update the driver version to 2.3.2.3
All applied, thanks.
^ permalink raw reply
* [PATCH] tg3: Do not use legacy PCI power management
From: Rafael J. Wysocki @ 2010-12-25 22:56 UTC (permalink / raw)
To: netdev; +Cc: Matt Carlson, Michael Chan, David Miller, Linux-pm mailing list
From: Rafael J. Wysocki <rjw@sisk.pl>
The tg3 driver uses the legacy PCI power management, so it has to do
some PCI-specific things in its ->suspend() and ->resume() callbacks,
which isn't necessary and should better be done by the PCI
sybsystem-level power management code.
Convert tg3 to the new PCI power management framework and make it
let the PCI subsystem take care of all the PCI-specific aspects of
device handling during system power transitions.
Tested on HP nx6325 with a NetXtreme BCM5788 adapter.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/net/tg3.c | 101 ++++++++++++++++++++++--------------------------------
1 file changed, 43 insertions(+), 58 deletions(-)
Index: linux-2.6/drivers/net/tg3.c
===================================================================
--- linux-2.6.orig/drivers/net/tg3.c
+++ linux-2.6/drivers/net/tg3.c
@@ -2549,39 +2549,35 @@ static void __tg3_set_mac_addr(struct tg
tw32(MAC_TX_BACKOFF_SEED, addr_high);
}
-static int tg3_set_power_state(struct tg3 *tp, pci_power_t state)
+static void tg3_enable_register_access(struct tg3 *tp)
{
- u32 misc_host_ctrl;
- bool device_should_wake, do_low_power;
-
- /* Make sure register accesses (indirect or otherwise)
- * will function correctly.
+ /*
+ * Make sure register accesses (indirect or otherwise) will function
+ * correctly.
*/
pci_write_config_dword(tp->pdev,
- TG3PCI_MISC_HOST_CTRL,
- tp->misc_host_ctrl);
+ TG3PCI_MISC_HOST_CTRL, tp->misc_host_ctrl);
+}
- switch (state) {
- case PCI_D0:
- pci_enable_wake(tp->pdev, state, false);
- pci_set_power_state(tp->pdev, PCI_D0);
+static int tg3_power_up(struct tg3 *tp)
+{
+ tg3_enable_register_access(tp);
- /* Switch out of Vaux if it is a NIC */
- if (tp->tg3_flags2 & TG3_FLG2_IS_NIC)
- tw32_wait_f(GRC_LOCAL_CTRL, tp->grc_local_ctrl, 100);
+ pci_set_power_state(tp->pdev, PCI_D0);
- return 0;
+ /* Switch out of Vaux if it is a NIC */
+ if (tp->tg3_flags2 & TG3_FLG2_IS_NIC)
+ tw32_wait_f(GRC_LOCAL_CTRL, tp->grc_local_ctrl, 100);
- case PCI_D1:
- case PCI_D2:
- case PCI_D3hot:
- break;
+ return 0;
+}
- default:
- netdev_err(tp->dev, "Invalid power state (D%d) requested\n",
- state);
- return -EINVAL;
- }
+static int tg3_power_down_prepare(struct tg3 *tp)
+{
+ u32 misc_host_ctrl;
+ bool device_should_wake, do_low_power;
+
+ tg3_enable_register_access(tp);
/* Restore the CLKREQ setting. */
if (tp->tg3_flags3 & TG3_FLG3_CLKREQ_BUG) {
@@ -2600,8 +2596,7 @@ static int tg3_set_power_state(struct tg
tw32(TG3PCI_MISC_HOST_CTRL,
misc_host_ctrl | MISC_HOST_CTRL_MASK_PCI_INT);
- device_should_wake = pci_pme_capable(tp->pdev, state) &&
- device_may_wakeup(&tp->pdev->dev) &&
+ device_should_wake = device_may_wakeup(&tp->pdev->dev) &&
(tp->tg3_flags & TG3_FLAG_WOL_ENABLE);
if (tp->tg3_flags3 & TG3_FLG3_USE_PHYLIB) {
@@ -2823,13 +2818,15 @@ static int tg3_set_power_state(struct tg
tg3_write_sig_post_reset(tp, RESET_KIND_SHUTDOWN);
- if (device_should_wake)
- pci_enable_wake(tp->pdev, state, true);
+ return 0;
+}
- /* Finally, set the new power state. */
- pci_set_power_state(tp->pdev, state);
+static void tg3_power_down(struct tg3 *tp)
+{
+ tg3_power_down_prepare(tp);
- return 0;
+ pci_wake_from_d3(tp->pdev, tp->tg3_flags & TG3_FLAG_WOL_ENABLE);
+ pci_set_power_state(tp->pdev, PCI_D3hot);
}
static void tg3_aux_stat_to_speed_duplex(struct tg3 *tp, u32 val, u16 *speed, u8 *duplex)
@@ -9101,7 +9098,7 @@ static int tg3_open(struct net_device *d
netif_carrier_off(tp->dev);
- err = tg3_set_power_state(tp, PCI_D0);
+ err = tg3_power_up(tp);
if (err)
return err;
@@ -9266,7 +9263,7 @@ static int tg3_close(struct net_device *
tg3_free_consistent(tp);
- tg3_set_power_state(tp, PCI_D3hot);
+ tg3_power_down(tp);
netif_carrier_off(tp->dev);
@@ -11068,7 +11065,7 @@ static void tg3_self_test(struct net_dev
struct tg3 *tp = netdev_priv(dev);
if (tp->phy_flags & TG3_PHYFLG_IS_LOW_POWER)
- tg3_set_power_state(tp, PCI_D0);
+ tg3_power_up(tp);
memset(data, 0, sizeof(u64) * TG3_NUM_TEST);
@@ -11136,7 +11133,7 @@ static void tg3_self_test(struct net_dev
tg3_phy_start(tp);
}
if (tp->phy_flags & TG3_PHYFLG_IS_LOW_POWER)
- tg3_set_power_state(tp, PCI_D3hot);
+ tg3_power_down(tp);
}
@@ -13546,7 +13543,7 @@ static int __devinit tg3_get_invariants(
(tp->tg3_flags3 & TG3_FLG3_5717_PLUS))
tp->tg3_flags |= TG3_FLAG_CPMU_PRESENT;
- /* Set up tp->grc_local_ctrl before calling tg3_set_power_state().
+ /* Set up tp->grc_local_ctrl before calling tg_power_up().
* GPIO1 driven high will bring 5700's external PHY out of reset.
* It is also used as eeprom write protect on LOMs.
*/
@@ -13577,7 +13574,7 @@ static int __devinit tg3_get_invariants(
}
/* Force the chip into D0. */
- err = tg3_set_power_state(tp, PCI_D0);
+ err = tg3_power_up(tp);
if (err) {
dev_err(&tp->pdev->dev, "Transition to D0 failed\n");
return err;
@@ -14980,19 +14977,13 @@ static void __devexit tg3_remove_one(str
}
}
-static int tg3_suspend(struct pci_dev *pdev, pm_message_t state)
+static int tg3_suspend(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct net_device *dev = pci_get_drvdata(pdev);
struct tg3 *tp = netdev_priv(dev);
- pci_power_t target_state;
int err;
- /* PCI register 4 needs to be saved whether netif_running() or not.
- * MSI address and data need to be saved if using MSI and
- * netif_running().
- */
- pci_save_state(pdev);
-
if (!netif_running(dev))
return 0;
@@ -15013,9 +15004,7 @@ static int tg3_suspend(struct pci_dev *p
tp->tg3_flags &= ~TG3_FLAG_INIT_COMPLETE;
tg3_full_unlock(tp);
- target_state = pdev->pm_cap ? pci_target_state(pdev) : PCI_D3hot;
-
- err = tg3_set_power_state(tp, target_state);
+ err = tg3_power_down_prepare(tp);
if (err) {
int err2;
@@ -15042,21 +15031,16 @@ out:
return err;
}
-static int tg3_resume(struct pci_dev *pdev)
+static int tg3_resume(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct net_device *dev = pci_get_drvdata(pdev);
struct tg3 *tp = netdev_priv(dev);
int err;
- pci_restore_state(tp->pdev);
-
if (!netif_running(dev))
return 0;
- err = tg3_set_power_state(tp, PCI_D0);
- if (err)
- return err;
-
netif_device_attach(dev);
tg3_full_lock(tp, 0);
@@ -15080,13 +15064,14 @@ out:
return err;
}
+static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
+
static struct pci_driver tg3_driver = {
.name = DRV_MODULE_NAME,
.id_table = tg3_pci_tbl,
.probe = tg3_init_one,
.remove = __devexit_p(tg3_remove_one),
- .suspend = tg3_suspend,
- .resume = tg3_resume
+ .driver.pm = &tg3_pm_ops,
};
static int __init tg3_init(void)
^ permalink raw reply
* Re: [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: Dan Carpenter @ 2010-12-25 22:46 UTC (permalink / raw)
To: Jesper Juhl
Cc: richard -rw- weinberger, ceph-devel, linux-kernel, netdev,
Sage Weil, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1012252223380.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 10:24:57PM +0100, Jesper Juhl wrote:
> On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
>
> > On Sat, Dec 25, 2010 at 7:17 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > > Hello,
> > >
> > > In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
> > > freed by kfree() and subsequently used in a call to dout() - use after
> > > free bug.
> >
> > Not really. %p reads only the address of "client".
> > kfree() does not alter this address.
> >
>
> Ok, I see your point and you are correct. But still, the patch does not
> change behaviour and it makes it absolutely clear that there's no
> use-after-free bug, so it might still have merit... or?
>
I see these with Smatch as well. This type of usage is quite common.
People do it deliberately and I guess they feel it's readable. Don't
change them.
If it were something that a static checker couldn't figure out, then
I'd say change it, but really the static checkers should just be made
smarter. Some day I'm going to make Smatch complain if it's a %s in
the string instead of a %p, but for now I just ignore the false
positives.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] CAN: Use inode instead of kernel address for /proc file
From: Dan Rosenberg @ 2010-12-25 22:41 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Urs Thuermann, David S. Miller, netdev, security
In-Reply-To: <4D1670CC.1000709@hartkopp.net>
>
> One minor question:
>
> AFAIK the inode numbers that can be found in /proc/<pid>/fd/* are in decimal
> and not in hex, right?
>
> If so, you should use '%lu' instead of '%lx' in the patch.
Yes, that's usually how they're expressed, but I did it this way for two
reasons. Firstly, %lu would require another change to the buffer size,
since the output could be up to 20 bytes long (plus another for the NULL
terminator). Secondly, by expressing it as hex it avoids breaking any
userland utilities that are expecting addresses, even if no such
utilities exist.
-Dan
^ permalink raw reply
* Re: [PATCH] CAN: Use inode instead of kernel address for /proc file
From: Oliver Hartkopp @ 2010-12-25 22:31 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: Urs Thuermann, David S. Miller, netdev, security
In-Reply-To: <4D166E9D.5080200@hartkopp.net>
On 25.12.2010 23:22, Oliver Hartkopp wrote:
> On 25.12.2010 23:16, Dan Rosenberg wrote:
>> Since the socket address is just being used as a unique identifier, its
>> inode number is an alternative that does not leak potentially sensitive
>> information.
>>
>> CC-ing stable because MITRE has assigned CVE-2010-4565 to the issue.
>>
>> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
>> Cc: stable <stable@kernel.org>
>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
> Thanks Dan.
>
>> ---
>> net/can/bcm.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>> index 6faa825..5748901 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
>> @@ -1520,8 +1520,8 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
>> bo->bound = 1;
>>
>> if (proc_dir) {
>> - /* unique socket address as filename */
>> - sprintf(bo->procname, "%p", sock);
>> + /* socket inode as filename */
>> + sprintf(bo->procname, "%lx", sock_i_ino(sk));
One minor question:
AFAIK the inode numbers that can be found in /proc/<pid>/fd/* are in decimal
and not in hex, right?
If so, you should use '%lu' instead of '%lx' in the patch.
Regards,
Oliver
^ permalink raw reply
* [patch] USB: cdc_ether: remove unneeded check
From: Dan Carpenter @ 2010-12-25 22:23 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb, netdev, kernel-janitors
We already verified that "dev->udev->actconfig->extralen" was non-zero
so "len" is non-zero here as well.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
Compile tested.
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index b3fe0de..9a60e41 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -99,9 +99,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
*/
buf = dev->udev->actconfig->extra;
len = dev->udev->actconfig->extralen;
- if (len)
- dev_dbg(&intf->dev,
- "CDC descriptors on config\n");
+ dev_dbg(&intf->dev, "CDC descriptors on config\n");
}
/* Maybe CDC descriptors are after the endpoint? This bug has
^ permalink raw reply related
* Re: [PATCH] CAN: Use inode instead of kernel address for /proc file
From: Oliver Hartkopp @ 2010-12-25 22:22 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: Urs Thuermann, David S. Miller, netdev, security
In-Reply-To: <1293315371.9764.44.camel@Dan>
On 25.12.2010 23:16, Dan Rosenberg wrote:
> Since the socket address is just being used as a unique identifier, its
> inode number is an alternative that does not leak potentially sensitive
> information.
>
> CC-ing stable because MITRE has assigned CVE-2010-4565 to the issue.
>
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Cc: stable <stable@kernel.org>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Thanks Dan.
> ---
> net/can/bcm.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 6faa825..5748901 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -1520,8 +1520,8 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
> bo->bound = 1;
>
> if (proc_dir) {
> - /* unique socket address as filename */
> - sprintf(bo->procname, "%p", sock);
> + /* socket inode as filename */
> + sprintf(bo->procname, "%lx", sock_i_ino(sk));
> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
> proc_dir,
> &bcm_proc_fops, sk);
>
^ permalink raw reply
* Re: [PATCH] 3c59x: Don't assign when a comparison is intended
From: Wolfram Sang @ 2010-12-25 22:17 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: Jesper Juhl, netdev, vortex, becker, Steffen Klassert,
linux-kernel
In-Reply-To: <AANLkTi=Y4wDG6+AY1=8NuFLGuCidgqaU1LginfB1YpZH@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
On Sat, Dec 25, 2010 at 09:50:56PM +0100, richard -rw- weinberger wrote:
> On Sat, Dec 25, 2010 at 9:30 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > Hi,
> >
> > In drivers/net/3c59x.c::vortex_probe1() we have this code:
> >
> > if (gendev) {
> > if ((pdev = DEVICE_PCI(gendev))) {
> > print_name = pci_name(pdev);
> > }
> >
> > if ((edev = DEVICE_EISA(gendev))) {
> > print_name = dev_name(&edev->dev);
> > }
> > }
> >
> > I believe these assignments were intended to be comparisons.
> > If I'm correct, then here's a patch to fix that up.
>
> I don't think so. Look at the extra brackets.
>
> The code can also written as:
>
> pdev = DEVICE_PCI(gendev);
> if(pdev)
> print_name = pci_name(pdev);
... which looks much better and could be worth a patch as well.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] CAN: Use inode instead of kernel address for /proc file
From: Dan Rosenberg @ 2010-12-25 22:16 UTC (permalink / raw)
To: Oliver Hartkopp, Urs Thuermann, David S. Miller; +Cc: netdev, security
Since the socket address is just being used as a unique identifier, its
inode number is an alternative that does not leak potentially sensitive
information.
CC-ing stable because MITRE has assigned CVE-2010-4565 to the issue.
Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: stable <stable@kernel.org>
---
net/can/bcm.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 6faa825..5748901 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1520,8 +1520,8 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
bo->bound = 1;
if (proc_dir) {
- /* unique socket address as filename */
- sprintf(bo->procname, "%p", sock);
+ /* socket inode as filename */
+ sprintf(bo->procname, "%lx", sock_i_ino(sk));
bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
proc_dir,
&bcm_proc_fops, sk);
^ permalink raw reply related
* Re: [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: richard -rw- weinberger @ 2010-12-25 21:40 UTC (permalink / raw)
To: Jesper Juhl; +Cc: ceph-devel, linux-kernel, netdev, Sage Weil, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1012252223380.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 10:24 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
>
>> On Sat, Dec 25, 2010 at 7:17 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> > Hello,
>> >
>> > In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
>> > freed by kfree() and subsequently used in a call to dout() - use after
>> > free bug.
>>
>> Not really. %p reads only the address of "client".
>> kfree() does not alter this address.
>>
>
> Ok, I see your point and you are correct. But still, the patch does not
> change behaviour and it makes it absolutely clear that there's no
> use-after-free bug, so it might still have merit... or?
Your patch does not fix a bug.
I would say it's a style fix.
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
>
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: Jesper Juhl @ 2010-12-25 21:35 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: ceph-devel, linux-kernel, netdev, Sage Weil, David S. Miller
In-Reply-To: <AANLkTincx8rSM+72czJ+tTrG=D=aOJLBb9nd9Jt_RsD2@mail.gmail.com>
On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
> On Sat, Dec 25, 2010 at 10:24 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
> >
> >> On Sat, Dec 25, 2010 at 7:17 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> >> > Hello,
> >> >
> >> > In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
> >> > freed by kfree() and subsequently used in a call to dout() - use after
> >> > free bug.
> >>
> >> Not really. %p reads only the address of "client".
> >> kfree() does not alter this address.
> >>
> >
> > Ok, I see your point and you are correct. But still, the patch does not
> > change behaviour and it makes it absolutely clear that there's no
> > use-after-free bug, so it might still have merit... or?
>
> Your patch does not fix a bug.
> I would say it's a style fix.
>
At this point in time I'd agree. :-)
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply
* Re: [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: Jesper Juhl @ 2010-12-25 21:24 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: ceph-devel, linux-kernel, netdev, Sage Weil, David S. Miller
In-Reply-To: <AANLkTik+yrpY8T69w_RQ84ZbRqwkjvFhvxHLBvU14N3f@mail.gmail.com>
On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
> On Sat, Dec 25, 2010 at 7:17 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > Hello,
> >
> > In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
> > freed by kfree() and subsequently used in a call to dout() - use after
> > free bug.
>
> Not really. %p reads only the address of "client".
> kfree() does not alter this address.
>
Ok, I see your point and you are correct. But still, the patch does not
change behaviour and it makes it absolutely clear that there's no
use-after-free bug, so it might still have merit... or?
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply
* Re: [PATCH] Ceph: Fix use-after-free bug in ceph_messenger_destroy()
From: richard -rw- weinberger @ 2010-12-25 21:14 UTC (permalink / raw)
To: Jesper Juhl; +Cc: ceph-devel, netdev, linux-kernel, David S. Miller, Sage Weil
In-Reply-To: <alpine.LNX.2.00.1012251908410.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 7:11 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> Hi,
>
> In net/ceph/messenger.c::ceph_messenger_destroy() the pointer 'msgr' is
> freed by kfree() and subsequently used in a call to dout() - use after
> free bug.
As I sad before, %p reads only the address.
> Easily fixed by simply moving the kfree() call after the dout() call.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> messenger.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index b6ff4a1..26514a7 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -2131,8 +2131,8 @@ void ceph_messenger_destroy(struct ceph_messenger *msgr)
> dout("destroy %p\n", msgr);
> kunmap(msgr->zero_page);
> __free_page(msgr->zero_page);
> - kfree(msgr);
> dout("destroyed messenger %p\n", msgr);
> + kfree(msgr);
> }
> EXPORT_SYMBOL(ceph_messenger_destroy);
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: richard -rw- weinberger @ 2010-12-25 21:12 UTC (permalink / raw)
To: Jesper Juhl; +Cc: ceph-devel, linux-kernel, netdev, Sage Weil, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1012251914370.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 7:17 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> Hello,
>
> In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
> freed by kfree() and subsequently used in a call to dout() - use after
> free bug.
Not really. %p reads only the address of "client".
kfree() does not alter this address.
> Easily fixed by simply moving the kfree() call after the dout() call.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> ceph_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
> index f3e4a13..890bbbf 100644
> --- a/net/ceph/ceph_common.c
> +++ b/net/ceph/ceph_common.c
> @@ -408,8 +408,8 @@ void ceph_destroy_client(struct ceph_client *client)
>
> ceph_destroy_options(client->options);
>
> - kfree(client);
> dout("destroy_client %p done\n", client);
> + kfree(client);
> }
> EXPORT_SYMBOL(ceph_destroy_client);
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] 3c59x: Don't assign when a comparison is intended
From: richard -rw- weinberger @ 2010-12-25 21:00 UTC (permalink / raw)
To: Jesper Juhl; +Cc: netdev, Steffen Klassert, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1012252144030.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 9:45 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
>
>> On Sat, Dec 25, 2010 at 9:30 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> > Hi,
>> >
>> > In drivers/net/3c59x.c::vortex_probe1() we have this code:
>> >
>> > if (gendev) {
>> > if ((pdev = DEVICE_PCI(gendev))) {
>> > print_name = pci_name(pdev);
>> > }
>> >
>> > if ((edev = DEVICE_EISA(gendev))) {
>> > print_name = dev_name(&edev->dev);
>> > }
>> > }
>> >
>> > I believe these assignments were intended to be comparisons.
>> > If I'm correct, then here's a patch to fix that up.
>>
>> I don't think so. Look at the extra brackets.
>>
>> The code can also written as:
>>
>> pdev = DEVICE_PCI(gendev);
>> if(pdev)
>> print_name = pci_name(pdev);
>>
>
> Arrgh, I completely missed that - damn.
> You are correct I think and my patch is wrong.
> Thanks for taking a look.
BTW: gcc is smart enough to catch such typos.
if(x = 3){
...
}
...would trigger a warning like this one:
"warning: suggest parentheses around assignment used as truth value"
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] 3c59x: Don't assign when a comparison is intended
From: richard -rw- weinberger @ 2010-12-25 20:50 UTC (permalink / raw)
To: Jesper Juhl; +Cc: netdev, vortex, becker, Steffen Klassert, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1012252125410.10759@swampdragon.chaosbits.net>
On Sat, Dec 25, 2010 at 9:30 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> Hi,
>
> In drivers/net/3c59x.c::vortex_probe1() we have this code:
>
> if (gendev) {
> if ((pdev = DEVICE_PCI(gendev))) {
> print_name = pci_name(pdev);
> }
>
> if ((edev = DEVICE_EISA(gendev))) {
> print_name = dev_name(&edev->dev);
> }
> }
>
> I believe these assignments were intended to be comparisons.
> If I'm correct, then here's a patch to fix that up.
I don't think so. Look at the extra brackets.
The code can also written as:
pdev = DEVICE_PCI(gendev);
if(pdev)
print_name = pci_name(pdev);
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> 3c59x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
> index 0a92436f..db8a80e 100644
> --- a/drivers/net/3c59x.c
> +++ b/drivers/net/3c59x.c
> @@ -1110,11 +1110,11 @@ static int __devinit vortex_probe1(struct device *gendev,
> }
>
> if (gendev) {
> - if ((pdev = DEVICE_PCI(gendev))) {
> + if ((pdev == DEVICE_PCI(gendev))) {
> print_name = pci_name(pdev);
> }
>
> - if ((edev = DEVICE_EISA(gendev))) {
> + if ((edev == DEVICE_EISA(gendev))) {
> print_name = dev_name(&edev->dev);
> }
> }
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] 3c59x: Don't assign when a comparison is intended
From: Jesper Juhl @ 2010-12-25 20:45 UTC (permalink / raw)
To: richard -rw- weinberger; +Cc: netdev, Steffen Klassert, linux-kernel
In-Reply-To: <AANLkTi=Y4wDG6+AY1=8NuFLGuCidgqaU1LginfB1YpZH@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1093 bytes --]
On Sat, 25 Dec 2010, richard -rw- weinberger wrote:
> On Sat, Dec 25, 2010 at 9:30 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > Hi,
> >
> > In drivers/net/3c59x.c::vortex_probe1() we have this code:
> >
> > if (gendev) {
> > if ((pdev = DEVICE_PCI(gendev))) {
> > print_name = pci_name(pdev);
> > }
> >
> > if ((edev = DEVICE_EISA(gendev))) {
> > print_name = dev_name(&edev->dev);
> > }
> > }
> >
> > I believe these assignments were intended to be comparisons.
> > If I'm correct, then here's a patch to fix that up.
>
> I don't think so. Look at the extra brackets.
>
> The code can also written as:
>
> pdev = DEVICE_PCI(gendev);
> if(pdev)
> print_name = pci_name(pdev);
>
Arrgh, I completely missed that - damn.
You are correct I think and my patch is wrong.
Thanks for taking a look.
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply
* [PATCH] 3c59x: Don't assign when a comparison is intended
From: Jesper Juhl @ 2010-12-25 20:30 UTC (permalink / raw)
To: netdev; +Cc: vortex, becker, Steffen Klassert, linux-kernel
Hi,
In drivers/net/3c59x.c::vortex_probe1() we have this code:
if (gendev) {
if ((pdev = DEVICE_PCI(gendev))) {
print_name = pci_name(pdev);
}
if ((edev = DEVICE_EISA(gendev))) {
print_name = dev_name(&edev->dev);
}
}
I believe these assignments were intended to be comparisons.
If I'm correct, then here's a patch to fix that up.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
3c59x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index 0a92436f..db8a80e 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -1110,11 +1110,11 @@ static int __devinit vortex_probe1(struct device *gendev,
}
if (gendev) {
- if ((pdev = DEVICE_PCI(gendev))) {
+ if ((pdev == DEVICE_PCI(gendev))) {
print_name = pci_name(pdev);
}
- if ((edev = DEVICE_EISA(gendev))) {
+ if ((edev == DEVICE_EISA(gendev))) {
print_name = dev_name(&edev->dev);
}
}
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* Re: [PATCH V7 1/8] ntp: add ADJ_SETOFFSET mode bit
From: Kuwahara,T. @ 2010-12-25 20:38 UTC (permalink / raw)
To: Richard Cochran
Cc: john stultz, linux-kernel, linux-api, netdev, Alan Cox,
Arnd Bergmann, Christoph Lameter, David Miller, Krzysztof Halasa,
Peter Zijlstra, Rodolfo Giometti, Thomas Gleixner
In-Reply-To: <20101223061359.GA7169@riccoc20.at.omicron.at>
After all, I'd prefer your earlier patchset. Leaving aside the
compatibility issue, there's no particular reason we have to re-use
the struct timex, which requires otherwise unnecessary conditional
branches as well as unit conversions. Don't you agree?
^ permalink raw reply
* [PATCH] Ceph: Fix a use-after-free bug in ceph_destroy_client().
From: Jesper Juhl @ 2010-12-25 18:17 UTC (permalink / raw)
To: ceph-devel; +Cc: linux-kernel, netdev, Sage Weil, David S. Miller
Hello,
In net/ceph/ceph_common.c::ceph_destroy_client() the pointer 'client' is
freed by kfree() and subsequently used in a call to dout() - use after
free bug.
Easily fixed by simply moving the kfree() call after the dout() call.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
ceph_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index f3e4a13..890bbbf 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -408,8 +408,8 @@ void ceph_destroy_client(struct ceph_client *client)
ceph_destroy_options(client->options);
- kfree(client);
dout("destroy_client %p done\n", client);
+ kfree(client);
}
EXPORT_SYMBOL(ceph_destroy_client);
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH] Ceph: Fix use-after-free bug in ceph_messenger_destroy()
From: Jesper Juhl @ 2010-12-25 18:11 UTC (permalink / raw)
To: ceph-devel; +Cc: netdev, linux-kernel, David S. Miller, Sage Weil
Hi,
In net/ceph/messenger.c::ceph_messenger_destroy() the pointer 'msgr' is
freed by kfree() and subsequently used in a call to dout() - use after
free bug.
Easily fixed by simply moving the kfree() call after the dout() call.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
messenger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index b6ff4a1..26514a7 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -2131,8 +2131,8 @@ void ceph_messenger_destroy(struct ceph_messenger *msgr)
dout("destroy %p\n", msgr);
kunmap(msgr->zero_page);
__free_page(msgr->zero_page);
- kfree(msgr);
dout("destroyed messenger %p\n", msgr);
+ kfree(msgr);
}
EXPORT_SYMBOL(ceph_messenger_destroy);
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH net-2.6 v2] epic100: hamachi: yellowfin: Fix skb allocation size
From: Jarek Poplawski @ 2010-12-25 17:39 UTC (permalink / raw)
To: David Miller
Cc: Joel Soete, Eric Dumazet, Andrew Morton, Linux Kernel, netdev
In-Reply-To: <20101225151217.GA1994@del.dom.local>
Joel Soete reported oopses during pppoe over sundance NIC, caused by
a bug in skb allocation and dma mapping code, where skb_reserve()
bytes weren't taken into account. As a followup to the patch:
"sundance: Fix oopses with corrupted skb_shared_info" very similar
code is fixed here for three other drivers.
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Cc: Joel Soete <soete.joel@scarlet.be>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
v2: a tiny changelog fix only
drivers/net/epic100.c | 4 ++--
drivers/net/hamachi.c | 4 ++--
drivers/net/yellowfin.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/epic100.c b/drivers/net/epic100.c
index aa56963..c353bf3 100644
--- a/drivers/net/epic100.c
+++ b/drivers/net/epic100.c
@@ -935,7 +935,7 @@ static void epic_init_ring(struct net_device *dev)
/* Fill in the Rx buffers. Handle allocation failure gracefully. */
for (i = 0; i < RX_RING_SIZE; i++) {
- struct sk_buff *skb = dev_alloc_skb(ep->rx_buf_sz);
+ struct sk_buff *skb = dev_alloc_skb(ep->rx_buf_sz + 2);
ep->rx_skbuff[i] = skb;
if (skb == NULL)
break;
@@ -1233,7 +1233,7 @@ static int epic_rx(struct net_device *dev, int budget)
entry = ep->dirty_rx % RX_RING_SIZE;
if (ep->rx_skbuff[entry] == NULL) {
struct sk_buff *skb;
- skb = ep->rx_skbuff[entry] = dev_alloc_skb(ep->rx_buf_sz);
+ skb = ep->rx_skbuff[entry] = dev_alloc_skb(ep->rx_buf_sz + 2);
if (skb == NULL)
break;
skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
diff --git a/drivers/net/hamachi.c b/drivers/net/hamachi.c
index 9a64858..80d25ed 100644
--- a/drivers/net/hamachi.c
+++ b/drivers/net/hamachi.c
@@ -1202,7 +1202,7 @@ static void hamachi_init_ring(struct net_device *dev)
}
/* Fill in the Rx buffers. Handle allocation failure gracefully. */
for (i = 0; i < RX_RING_SIZE; i++) {
- struct sk_buff *skb = dev_alloc_skb(hmp->rx_buf_sz);
+ struct sk_buff *skb = dev_alloc_skb(hmp->rx_buf_sz + 2);
hmp->rx_skbuff[i] = skb;
if (skb == NULL)
break;
@@ -1669,7 +1669,7 @@ static int hamachi_rx(struct net_device *dev)
entry = hmp->dirty_rx % RX_RING_SIZE;
desc = &(hmp->rx_ring[entry]);
if (hmp->rx_skbuff[entry] == NULL) {
- struct sk_buff *skb = dev_alloc_skb(hmp->rx_buf_sz);
+ struct sk_buff *skb = dev_alloc_skb(hmp->rx_buf_sz + 2);
hmp->rx_skbuff[entry] = skb;
if (skb == NULL)
diff --git a/drivers/net/yellowfin.c b/drivers/net/yellowfin.c
index cd1b3dc..ec47e22 100644
--- a/drivers/net/yellowfin.c
+++ b/drivers/net/yellowfin.c
@@ -744,7 +744,7 @@ static int yellowfin_init_ring(struct net_device *dev)
}
for (i = 0; i < RX_RING_SIZE; i++) {
- struct sk_buff *skb = dev_alloc_skb(yp->rx_buf_sz);
+ struct sk_buff *skb = dev_alloc_skb(yp->rx_buf_sz + 2);
yp->rx_skbuff[i] = skb;
if (skb == NULL)
break;
@@ -1157,7 +1157,7 @@ static int yellowfin_rx(struct net_device *dev)
for (; yp->cur_rx - yp->dirty_rx > 0; yp->dirty_rx++) {
entry = yp->dirty_rx % RX_RING_SIZE;
if (yp->rx_skbuff[entry] == NULL) {
- struct sk_buff *skb = dev_alloc_skb(yp->rx_buf_sz);
+ struct sk_buff *skb = dev_alloc_skb(yp->rx_buf_sz + 2);
if (skb == NULL)
break; /* Better luck next round. */
yp->rx_skbuff[entry] = skb;
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox