2013-05-07 14:21:33

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 01/15] Char: lp, protect LPGETSTATUS with port_mutex

From: "[email protected]" <[email protected]>

The patch fixes a problem in the lp driver that can cause oopses as
follows:
process A: calls lp_write, which in turn calls
parport_ieee1284_write_compat, and that invokes
parport_wait_peripheral
process B: meanwhile does an ioctl(LPGETSTATUS), which call
lp_release_parport when done. This function will set
physport->cad = NULL.
process A: parport_wait_peripheral tries to dereference
physport->cad and dies

So, protect that code with the port_mutex in order to protect against
simultaneous calls to lp_read/lp_write.

Similar protection is probably required for ioctl(LPRESET)...

This patch was done by IBM a while back and we (at suse) have that
since at least 2004 in our repos. Let's make it upstream.

Signed-off-by: [email protected]
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/char/lp.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index dafd9ac..0913d79 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -622,9 +622,12 @@ static int lp_do_ioctl(unsigned int minor, unsigned int cmd,
return -EFAULT;
break;
case LPGETSTATUS:
+ if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
+ return -EINTR;
lp_claim_parport_or_block (&lp_table[minor]);
status = r_str(minor);
lp_release_parport (&lp_table[minor]);
+ mutex_unlock(&lp_table[minor].port_mutex);

if (copy_to_user(argp, &status, sizeof(int)))
return -EFAULT;
--
1.8.2.1


2013-05-07 14:18:15

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

From: Jeff Mahoney <[email protected]>

The PCH EG20T is only compatible with Intel Atom processors so it
should depend on x86.

Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: [email protected]
---
drivers/ptp/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 1ea6f1d..e02b7d4 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -72,6 +72,7 @@ config DP83640_PHY

config PTP_1588_CLOCK_PCH
tristate "Intel PCH EG20T as PTP clock"
+ depends on X86
select PTP_1588_CLOCK
help
This driver adds support for using the PCH EG20T as a PTP
--
1.8.2.1

2013-05-07 14:18:13

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 04/15] drm/cirrus: Correct register values for 16bpp

From: Takashi Iwai <[email protected]>

When the mode is set with 16bpp on QEMU, the output gets totally
broken. The culprit is the bogus register values set for 16bpp,
which was likely copied from from a wrong place.

References: https://bugzilla.novell.com/show_bug.cgi?id=799216
Cc: <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: David Airlie <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/cirrus/cirrus_mode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 60685b21..379a47e 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -273,8 +273,8 @@ static int cirrus_crtc_mode_set(struct drm_crtc *crtc,
sr07 |= 0x11;
break;
case 16:
- sr07 |= 0xc1;
- hdr = 0xc0;
+ sr07 |= 0x17;
+ hdr = 0xc1;
break;
case 24:
sr07 |= 0x15;
--
1.8.2.1

2013-05-07 14:18:11

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 07/15] hfs: avoid crash in hfs_bnode_create

From: Jeff Mahoney <[email protected]>

Commit 634725a92938b0f282b17cec0b007dca77adebd2 removed the BUG_ON in
hfs_bnode_create in hfsplus. This patch removes it from the hfs
version and avoids an fsfuzzer crash.

Signed-off-by: Jeff Mahoney <[email protected]>
Acked-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
---
fs/hfs/bnode.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index f3b1a15..6d435c2 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -415,7 +415,11 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
spin_lock(&tree->hash_lock);
node = hfs_bnode_findhash(tree, num);
spin_unlock(&tree->hash_lock);
- BUG_ON(node);
+ if (node) {
+ printk(KERN_CRIT "new node %u already hashed?\n", num);
+ WARN_ON(1);
+ return node;
+ }
node = __hfs_bnode_create(tree, num);
if (!node)
return ERR_PTR(-ENOMEM);
--
1.8.2.1

2013-05-07 14:18:09

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 14/15] staging/sb105x: remove asm/segment.h dependency

From: Jeff Mahoney <[email protected]>

sb105x doesn't seem to actually need <asm/segment.h> (builds on x86
without it) and ppc/ppc64 doesn't provide it so it fails to build there.

This patch removes the dependency.

CC: Steven Rostedt <[email protected]>
Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/staging/sb105x/sb_pci_mp.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/sb105x/sb_pci_mp.h b/drivers/staging/sb105x/sb_pci_mp.h
index a15f470..11d9299 100644
--- a/drivers/staging/sb105x/sb_pci_mp.h
+++ b/drivers/staging/sb105x/sb_pci_mp.h
@@ -19,7 +19,6 @@
#include <asm/uaccess.h>
#include <asm/io.h>
#include <asm/irq.h>
-#include <asm/segment.h>
#include <asm/serial.h>
#include <linux/interrupt.h>

--
1.8.2.1

2013-05-07 14:19:09

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 10/15] acpi: ec_sys: access user space with get_user()/put_user()

From: Vasiliy Kulikov <[email protected]>

User space pointer may not be dereferenced. Use get_user()/put_user()
instead and check their return codes.

Signed-off-by: Vasiliy Kulikov <[email protected]>
Signed-off-by: Thomas Renninger <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Len Brown <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: [email protected]
---
drivers/acpi/ec_sys.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/ec_sys.c b/drivers/acpi/ec_sys.c
index 7586544..4e7b798 100644
--- a/drivers/acpi/ec_sys.c
+++ b/drivers/acpi/ec_sys.c
@@ -12,6 +12,7 @@
#include <linux/acpi.h>
#include <linux/debugfs.h>
#include <linux/module.h>
+#include <linux/uaccess.h>
#include "internal.h"

MODULE_AUTHOR("Thomas Renninger <[email protected]>");
@@ -34,7 +35,6 @@ static ssize_t acpi_ec_read_io(struct file *f, char __user *buf,
* struct acpi_ec *ec = ((struct seq_file *)f->private_data)->private;
*/
unsigned int size = EC_SPACE_SIZE;
- u8 *data = (u8 *) buf;
loff_t init_off = *off;
int err = 0;

@@ -47,9 +47,15 @@ static ssize_t acpi_ec_read_io(struct file *f, char __user *buf,
size = count;

while (size) {
- err = ec_read(*off, &data[*off - init_off]);
+ u8 byte_read;
+ err = ec_read(*off, &byte_read);
if (err)
return err;
+ if (put_user(byte_read, buf + *off - init_off)) {
+ if (*off - init_off)
+ return *off - init_off; /* partial read */
+ return -EFAULT;
+ }
*off += 1;
size--;
}
@@ -65,7 +71,6 @@ static ssize_t acpi_ec_write_io(struct file *f, const char __user *buf,

unsigned int size = count;
loff_t init_off = *off;
- u8 *data = (u8 *) buf;
int err = 0;

if (*off >= EC_SPACE_SIZE)
@@ -76,7 +81,12 @@ static ssize_t acpi_ec_write_io(struct file *f, const char __user *buf,
}

while (size) {
- u8 byte_write = data[*off - init_off];
+ u8 byte_write;
+ if (get_user(byte_write, buf + *off - init_off)) {
+ if (*off - init_off)
+ return *off - init_off; /* partial write */
+ return -EFAULT;
+ }
err = ec_write(*off, byte_write);
if (err)
return err;
--
1.8.2.1

2013-05-07 14:19:08

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 09/15] Make selection of 'readdir-plus' adapt to usage patterns.

From: NeilBrown <[email protected]>

While the use of READDIRPLUS is significantly more efficient than
READDIR followed by many GETATTR calls, it is still less efficient
than just READDIR if the attributes are not required.

We can get a hint as to whether the application requires attr information
by looking at whether any ->getattr calls are made between
->readdir calls.
If there are any, then getting the attributes seems to be worth while.

This patch tracks whether there have been recent getattr calls on
children of a directory and uses that information to selectively
disable READDIRPLUS on that directory.

The first 'readdir' call is always served using READDIRPLUS.
Subsequent calls only use READDIRPLUS if there was a getattr on a child
in the mean time.

The locking of ->d_parent access needs to be reviewed.
As the bit is simply a hint, it isn't critical that it is set
on the "correct" parent if a rename is happening, but it is
critical that the 'set' doesn't set a bit in something that
isn't even an inode any more.

Acked-by: NeilBrown <[email protected]>
Signed-off-by: Neil Brown <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Trond Myklebust <[email protected]>
Cc: [email protected]
---
fs/nfs/dir.c | 3 +++
fs/nfs/inode.c | 9 +++++++++
include/linux/nfs_fs.h | 4 ++++
3 files changed, 16 insertions(+)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e093e73..f5b367c 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -826,6 +826,9 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
desc->dir_cookie = &dir_ctx->dir_cookie;
desc->decode = NFS_PROTO(inode)->decode_dirent;
desc->plus = nfs_use_readdirplus(inode, filp) ? 1 : 0;
+ if (filp->f_pos > 0 && !test_bit(NFS_INO_SEEN_GETATTR, &NFS_I(inode)->flags))
+ desc->plus = 0;
+ clear_bit(NFS_INO_SEEN_GETATTR, &NFS_I(inode)->flags);

nfs_block_sillyrename(dentry);
res = nfs_revalidate_mapping(inode, filp->f_mapping);
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index c1c7a9d..c15071d 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -520,6 +520,15 @@ int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
struct inode *inode = dentry->d_inode;
int need_atime = NFS_I(inode)->cache_validity & NFS_INO_INVALID_ATIME;
int err;
+ struct dentry *p;
+ struct inode *pi;
+
+ rcu_read_lock();
+ p = dentry->d_parent;
+ pi = rcu_dereference(p)->d_inode;
+ if (pi && !test_bit(NFS_INO_SEEN_GETATTR, &NFS_I(pi)->flags))
+ set_bit(NFS_INO_SEEN_GETATTR, &NFS_I(pi)->flags);
+ rcu_read_unlock();

/* Flush out writes to the server in order to update c/mtime. */
if (S_ISREG(inode->i_mode)) {
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index fc01d5c..633e2a9 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -220,6 +220,10 @@ struct nfs_inode {
#define NFS_INO_COMMIT (7) /* inode is committing unstable writes */
#define NFS_INO_LAYOUTCOMMIT (9) /* layoutcommit required */
#define NFS_INO_LAYOUTCOMMITTING (10) /* layoutcommit inflight */
+#define NFS_INO_SEEN_GETATTR (11) /* flag to track if app is calling
+ * getattr in a directory during
+ * readdir
+ */

static inline struct nfs_inode *NFS_I(const struct inode *inode)
{
--
1.8.2.1

2013-05-07 14:18:06

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 11/15] nouveau: Make vga_switcheroo code depend on VGA_SWITCHEROO

From: Jeff Mahoney <[email protected]>

Commit 8116188fdef5946bcbb2d73e41d7412a57ffb034 (nouveau/acpi: hook up
to the MXM method for mux switching.) broke the build on non-x86
architectures due to the new dependency on MXM and MXM being an x86
platform driver.

It built previously since the vga switcheroo registration routines
were zereod out on !X86. The code was built in but unused.

This patch makes all of the DSM code depend on CONFIG_VGA_SWITCHEROO,
allowing it to build on non-x86 and shrinking the module size as well.

Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: David Airlie <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/nouveau/nouveau_acpi.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index d97f200..4d70fb7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -46,6 +46,7 @@ bool nouveau_is_v1_dsm(void) {
#define NOUVEAU_DSM_HAS_MUX 0x1
#define NOUVEAU_DSM_HAS_OPT 0x2

+#ifdef CONFIG_VGA_SWITCHEROO
static const char nouveau_dsm_muid[] = {
0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
@@ -337,6 +338,10 @@ void nouveau_unregister_dsm_handler(void)
if (nouveau_dsm_priv.optimus_detected || nouveau_dsm_priv.dsm_detected)
vga_switcheroo_unregister_handler();
}
+#else
+void nouveau_register_dsm_handler(void) {}
+void nouveau_unregister_dsm_handler(void) {}
+#endif

/* retrieve the ROM in 4k blocks */
static int nouveau_rom_call(acpi_handle rom_handle, uint8_t *bios,
--
1.8.2.1

2013-05-07 14:19:44

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 12/15] geodefb: Depend on X86_32

From: Jeff Mahoney <[email protected]>

Geode is 32-bit only so only enable it for X86_32.

Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Florian Tobias Schandinat <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/video/geode/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/geode/Kconfig b/drivers/video/geode/Kconfig
index 21e351a..7d8dcb4 100644
--- a/drivers/video/geode/Kconfig
+++ b/drivers/video/geode/Kconfig
@@ -3,7 +3,7 @@
#
config FB_GEODE
bool "AMD Geode family framebuffer support"
- depends on FB && PCI && X86
+ depends on FB && PCI && X86_32
---help---
Say 'Y' here to allow you to select framebuffer drivers for
the AMD Geode family of processors.
--
1.8.2.1

2013-05-07 14:19:43

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 05/15] connection tracking helper for SLP

From: Jiri Bohac <[email protected]>

A simple connection tracking helper for SLP. Marks replies to a
SLP broadcast query as ESTABLISHED to allow them to pass through the
firewall.

Signed-off-by: Jiri Bohac <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: "David S. Miller" <[email protected]>
Cc: Patrick McHardy <[email protected]>
Cc: Pablo Neira Ayuso <[email protected]>
---
net/netfilter/Kconfig | 15 +++++
net/netfilter/Makefile | 1 +
net/netfilter/nf_conntrack_slp.c | 131 +++++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 net/netfilter/nf_conntrack_slp.c

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 56d22ca..ec61b30 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -320,6 +320,21 @@ config NF_CONNTRACK_TFTP

To compile it as a module, choose M here. If unsure, say N.

+config NF_CONNTRACK_SLP
+ tristate "SLP protocol support"
+ depends on NF_CONNTRACK
+ depends on NETFILTER_ADVANCED
+ help
+ SLP queries are sometimes sent as broadcast messages from an
+ unprivileged port and responded to with unicast messages to the
+ same port. This make them hard to firewall properly because connection
+ tracking doesn't deal with broadcasts. This helper tracks locally
+ originating broadcast SLP queries and the corresponding
+ responses. It relies on correct IP address configuration, specifically
+ netmask and broadcast address.
+
+ To compile it as a module, choose M here. If unsure, say N.
+
config NF_CT_NETLINK
tristate 'Connection tracking netlink interface'
select NETFILTER_NETLINK
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index a1abf87..aa7d5f1 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_NF_CONNTRACK_PPTP) += nf_conntrack_pptp.o
obj-$(CONFIG_NF_CONNTRACK_SANE) += nf_conntrack_sane.o
obj-$(CONFIG_NF_CONNTRACK_SIP) += nf_conntrack_sip.o
obj-$(CONFIG_NF_CONNTRACK_TFTP) += nf_conntrack_tftp.o
+obj-$(CONFIG_NF_CONNTRACK_SLP) += nf_conntrack_slp.o

nf_nat-y := nf_nat_core.o nf_nat_proto_unknown.o nf_nat_proto_common.o \
nf_nat_proto_udp.o nf_nat_proto_tcp.o nf_nat_helper.o
diff --git a/net/netfilter/nf_conntrack_slp.c b/net/netfilter/nf_conntrack_slp.c
new file mode 100644
index 0000000..0174dd0
--- /dev/null
+++ b/net/netfilter/nf_conntrack_slp.c
@@ -0,0 +1,131 @@
+/*
+ * NetBIOS name service broadcast connection tracking helper
+ *
+ * (c) 2007 Jiri Bohac <[email protected]>
+ * (c) 2005 Patrick McHardy <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+/*
+ * This helper tracks locally originating NetBIOS name service
+ * requests by issuing permanent expectations (valid until
+ * timing out) matching all reply connections from the
+ * destination network. The only NetBIOS specific thing is
+ * actually the port number.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/inetdevice.h>
+#include <linux/if_addr.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/netfilter.h>
+#include <net/route.h>
+
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_helper.h>
+#include <net/netfilter/nf_conntrack_expect.h>
+
+#define SLP_PORT 427
+
+MODULE_AUTHOR("Jiri Bohac <[email protected]>");
+MODULE_DESCRIPTION("SLP broadcast connection tracking helper");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ip_conntrack_slp");
+
+static unsigned int timeout __read_mostly = 3;
+module_param(timeout, uint, 0400);
+MODULE_PARM_DESC(timeout, "timeout for master connection/replies in seconds");
+
+static int help(struct sk_buff *skb, unsigned int protoff,
+ struct nf_conn *ct, enum ip_conntrack_info ctinfo)
+{
+ struct nf_conntrack_expect *exp;
+ struct rtable *rt = skb_rtable(skb);
+ struct in_device *in_dev;
+ __be32 mask = 0;
+ __be32 src = 0;
+
+ /* we're only interested in locally generated packets */
+ if (skb->sk == NULL)
+ goto out;
+ if (rt == NULL || !(rt->rt_flags & (RTCF_MULTICAST|RTCF_BROADCAST)))
+ goto out;
+ if (CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL)
+ goto out;
+
+ rcu_read_lock();
+ in_dev = __in_dev_get_rcu(rt->dst.dev);
+ if (in_dev != NULL) {
+ for_primary_ifa(in_dev) {
+ /* this is a hack as slp uses multicast we can't match
+ * the destination address to some broadcast address. So
+ * just take the first one. Better would be to install
+ * expectations for all addresses */
+ mask = ifa->ifa_mask;
+ src = ifa->ifa_broadcast;
+ break;
+ } endfor_ifa(in_dev);
+ }
+ rcu_read_unlock();
+
+ if (mask == 0 || src == 0)
+ goto out;
+
+ exp = nf_ct_expect_alloc(ct);
+ if (exp == NULL)
+ goto out;
+
+ exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+ exp->tuple.src.u3.ip = src;
+ exp->tuple.src.u.udp.port = htons(SLP_PORT);
+
+ exp->mask.src.u3.ip = mask;
+ exp->mask.src.u.udp.port = htons(0xFFFF);
+
+ exp->expectfn = NULL;
+ exp->flags = NF_CT_EXPECT_PERMANENT;
+ exp->class = NF_CT_EXPECT_CLASS_DEFAULT;
+ exp->helper = NULL;
+
+ nf_ct_expect_related(exp);
+ nf_ct_expect_put(exp);
+
+ nf_ct_refresh(ct, skb, timeout * HZ);
+out:
+ return NF_ACCEPT;
+}
+
+static struct nf_conntrack_expect_policy exp_policy = {
+ .max_expected = 1,
+};
+
+static struct nf_conntrack_helper helper __read_mostly = {
+ .name = "slp",
+ .tuple.src.l3num = AF_INET,
+ .tuple.src.u.udp.port = __constant_htons(SLP_PORT),
+ .tuple.dst.protonum = IPPROTO_UDP,
+ .me = THIS_MODULE,
+ .help = help,
+ .expect_policy = &exp_policy,
+};
+
+static int __init nf_conntrack_slp_init(void)
+{
+ exp_policy.timeout = timeout;
+ return nf_conntrack_helper_register(&helper);
+}
+
+static void __exit nf_conntrack_slp_fini(void)
+{
+ nf_conntrack_helper_unregister(&helper);
+}
+
+module_init(nf_conntrack_slp_init);
+module_exit(nf_conntrack_slp_fini);
--
1.8.2.1

2013-05-07 14:20:43

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 06/15] netfilter: Implement RFC 1123 for FTP conntrack

From: Jeff Mahoney <[email protected]>

The FTP conntrack code currently only accepts the following format for
the 227 response for PASV:
227 Entering Passive Mode (148,100,81,40,31,161).

It doesn't accept the following format from an obscure server:
227 Data transfer will passively listen to 67,218,99,134,50,144

From RFC 1123:
The format of the 227 reply to a PASV command is not
well standardized. In particular, an FTP client cannot
assume that the parentheses shown on page 40 of RFC-959
will be present (and in fact, Figure 3 on page 43 omits
them). Therefore, a User-FTP program that interprets
the PASV reply must scan the reply for the first digit
of the host and port numbers.

This patch adds support for the RFC 1123 clarification by:
- Allowing a search filter to specify NUL as the terminator so that
try_number will return successfully if the array of numbers has been
filled when an unexpected character is encountered.
- Using space as the separator for the 227 reply and then scanning for
the first digit of the number sequence. The number sequence is parsed
out using the existing try_rfc959 but with a NUL terminator.

References: https://bugzilla.novell.com/show_bug.cgi?id=466279
References: http://bugzilla.netfilter.org/show_bug.cgi?id=574
Reported-by: Mark Post <[email protected]>
Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Pablo Neira Ayuso <[email protected]>
Cc: Patrick McHardy <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
net/netfilter/nf_conntrack_ftp.c | 73 +++++++++++++++++++++++++++++-----------
1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index 6b21707..b8a0924 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -55,10 +55,14 @@ unsigned int (*nf_nat_ftp_hook)(struct sk_buff *skb,
struct nf_conntrack_expect *exp);
EXPORT_SYMBOL_GPL(nf_nat_ftp_hook);

-static int try_rfc959(const char *, size_t, struct nf_conntrack_man *, char);
-static int try_eprt(const char *, size_t, struct nf_conntrack_man *, char);
+static int try_rfc959(const char *, size_t, struct nf_conntrack_man *,
+ char, unsigned int *);
+static int try_rfc1123(const char *, size_t, struct nf_conntrack_man *,
+ char, unsigned int *);
+static int try_eprt(const char *, size_t, struct nf_conntrack_man *,
+ char, unsigned int *);
static int try_epsv_response(const char *, size_t, struct nf_conntrack_man *,
- char);
+ char, unsigned int *);

static struct ftp_search {
const char *pattern;
@@ -66,7 +70,7 @@ static struct ftp_search {
char skip;
char term;
enum nf_ct_ftp_type ftptype;
- int (*getnum)(const char *, size_t, struct nf_conntrack_man *, char);
+ int (*getnum)(const char *, size_t, struct nf_conntrack_man *, char, unsigned int *);
} search[IP_CT_DIR_MAX][2] = {
[IP_CT_DIR_ORIGINAL] = {
{
@@ -90,10 +94,8 @@ static struct ftp_search {
{
.pattern = "227 ",
.plen = sizeof("227 ") - 1,
- .skip = '(',
- .term = ')',
.ftptype = NF_CT_FTP_PASV,
- .getnum = try_rfc959,
+ .getnum = try_rfc1123,
},
{
.pattern = "229 ",
@@ -132,8 +134,9 @@ static int try_number(const char *data, size_t dlen, u_int32_t array[],
i++;
else {
/* Unexpected character; true if it's the
- terminator and we're finished. */
- if (*data == term && i == array_size - 1)
+ terminator (or we don't care about one)
+ and we're finished. */
+ if ((*data == term || !term) && i == array_size - 1)
return len;

pr_debug("Char %u (got %u nums) `%u' unexpected\n",
@@ -148,7 +151,8 @@ static int try_number(const char *data, size_t dlen, u_int32_t array[],

/* Returns 0, or length of numbers: 192,168,1,1,5,6 */
static int try_rfc959(const char *data, size_t dlen,
- struct nf_conntrack_man *cmd, char term)
+ struct nf_conntrack_man *cmd, char term,
+ unsigned int *offset)
{
int length;
u_int32_t array[6];
@@ -163,6 +167,33 @@ static int try_rfc959(const char *data, size_t dlen,
return length;
}

+/*
+ * From RFC 1123:
+ * The format of the 227 reply to a PASV command is not
+ * well standardized. In particular, an FTP client cannot
+ * assume that the parentheses shown on page 40 of RFC-959
+ * will be present (and in fact, Figure 3 on page 43 omits
+ * them). Therefore, a User-FTP program that interprets
+ * the PASV reply must scan the reply for the first digit
+ * of the host and port numbers.
+ */
+static int try_rfc1123(const char *data, size_t dlen,
+ struct nf_conntrack_man *cmd, char term,
+ unsigned int *offset)
+{
+ int i;
+ for (i = 0; i < dlen; i++)
+ if (isdigit(data[i]))
+ break;
+
+ if (i == dlen)
+ return 0;
+
+ *offset += i;
+
+ return try_rfc959(data + i, dlen - i, cmd, 0, offset);
+}
+
/* Grab port: number up to delimiter */
static int get_port(const char *data, int start, size_t dlen, char delim,
__be16 *port)
@@ -191,7 +222,7 @@ static int get_port(const char *data, int start, size_t dlen, char delim,

/* Returns 0, or length of numbers: |1|132.235.1.2|6275| or |2|3ffe::1|6275| */
static int try_eprt(const char *data, size_t dlen, struct nf_conntrack_man *cmd,
- char term)
+ char term, unsigned int *offset)
{
char delim;
int length;
@@ -239,7 +270,8 @@ static int try_eprt(const char *data, size_t dlen, struct nf_conntrack_man *cmd,

/* Returns 0, or length of numbers: |||6446| */
static int try_epsv_response(const char *data, size_t dlen,
- struct nf_conntrack_man *cmd, char term)
+ struct nf_conntrack_man *cmd, char term,
+ unsigned int *offset)
{
char delim;

@@ -261,9 +293,10 @@ static int find_pattern(const char *data, size_t dlen,
unsigned int *numlen,
struct nf_conntrack_man *cmd,
int (*getnum)(const char *, size_t,
- struct nf_conntrack_man *, char))
+ struct nf_conntrack_man *, char,
+ unsigned int *))
{
- size_t i;
+ size_t i = plen;

pr_debug("find_pattern `%s': dlen = %Zu\n", pattern, dlen);
if (dlen == 0)
@@ -293,16 +326,18 @@ static int find_pattern(const char *data, size_t dlen,
pr_debug("Pattern matches!\n");
/* Now we've found the constant string, try to skip
to the 'skip' character */
- for (i = plen; data[i] != skip; i++)
- if (i == dlen - 1) return -1;
+ if (skip) {
+ for (i = plen; data[i] != skip; i++)
+ if (i == dlen - 1) return -1;

- /* Skip over the last character */
- i++;
+ /* Skip over the last character */
+ i++;
+ }

pr_debug("Skipped up to `%c'!\n", skip);

*numoff = i;
- *numlen = getnum(data + i, dlen - i, cmd, term);
+ *numlen = getnum(data + i, dlen - i, cmd, term, numoff);
if (!*numlen)
return -1;

--
1.8.2.1

2013-05-07 14:20:42

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 03/15] ehea: add alias entry for portN properties

From: Olaf Hering <[email protected]>

Use separate table for alias entries in the ehea module,
otherwise the probe() function will operate on the separate ports
instead of the lhea-"root" entry of the device-tree

References: https://bugzilla.novell.com/show_bug.cgi?id=435215
Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Olaf Hering <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Thadeu Lima de Souza Cascardo <[email protected]>
Cc: [email protected]
Cc: "David S. Miller" <[email protected]>
---
drivers/net/ethernet/ibm/ehea/ehea_main.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 90ea0b1..1114418 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -103,6 +103,19 @@ static int ehea_probe_adapter(struct platform_device *dev,

static int ehea_remove(struct platform_device *dev);

+static struct of_device_id ehea_module_device_table[] = {
+ {
+ .name = "lhea",
+ .compatible = "IBM,lhea",
+ },
+ {
+ .type = "network",
+ .compatible = "IBM,lhea-ethernet",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ehea_module_device_table);
+
static struct of_device_id ehea_device_table[] = {
{
.name = "lhea",
@@ -110,7 +123,6 @@ static struct of_device_id ehea_device_table[] = {
},
{},
};
-MODULE_DEVICE_TABLE(of, ehea_device_table);

static struct of_platform_driver ehea_driver = {
.driver = {
--
1.8.2.1

2013-05-07 14:21:10

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 08/15] reiserfs: fix deadlock with nfs racing on create/lookup

From: Jeff Mahoney <[email protected]>

Reiserfs is currently able to be deadlocked by having two NFS clients
where one has removed and recreated a file and another is accessing the
file with an open file handle.

If one client deletes and recreates a file with timing such that the
recreated file obtains the same [dirid, objectid] pair as the original
file while another client accesses the file via file handle, the create
and lookup can race and deadlock if the lookup manages to create the
in-memory inode first.

The create thread, in insert_inode_locked4, will hold the write lock
while waiting on the other inode to be unlocked. The lookup thread,
anywhere in the iget path, will release and reacquire the write lock while
it schedules. If it needs to reacquire the lock while the create thread
has it, it will never be able to make forward progress because it needs
to reacquire the lock before ultimately unlocking the inode.

This patch drops the write lock across the insert_inode_locked4 call so
that the ordering of inode_wait -> write lock is retained. Since this
would have been the case before the BKL push-down, this is safe.

Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
---
fs/reiserfs/inode.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index 77d6d47..f844533 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1811,11 +1811,16 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
TYPE_STAT_DATA, SD_SIZE, MAX_US_INT);
memcpy(INODE_PKEY(inode), &(ih.ih_key), KEY_SIZE);
args.dirid = le32_to_cpu(ih.ih_key.k_dir_id);
- if (insert_inode_locked4(inode, args.objectid,
- reiserfs_find_actor, &args) < 0) {
+
+ reiserfs_write_unlock(inode->i_sb);
+ err = insert_inode_locked4(inode, args.objectid,
+ reiserfs_find_actor, &args);
+ reiserfs_write_lock(inode->i_sb);
+ if (err) {
err = -EINVAL;
goto out_bad_inode;
}
+
if (old_format_only(sb))
/* not a perfect generation count, as object ids can be reused, but
** this is as good as reiserfs can do right now.
--
1.8.2.1

2013-05-07 14:21:34

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 02/15] mISDN: Add support for group membership check

From: Jeff Mahoney <[email protected]>

This patch adds a module parameter to allow a group access to the
mISDN devices. Otherwise, unpriviledged users on systems with ISDN
hardware have the ability to dial out, potentially causing expensive
bills.

Based on a different implementation by Patrick Koppen <[email protected]>

We (at suse) have this patch in our trees at least since 2009 and want
to push it upstream now.

Acked-by: Jeff Mahoney <[email protected]>
Cc: Patrick Koppen <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Karsten Keil <[email protected]>
Cc: [email protected]
Cc: "David S. Miller" <[email protected]>
---
drivers/isdn/mISDN/core.c | 3 +++
drivers/isdn/mISDN/core.h | 1 +
drivers/isdn/mISDN/socket.c | 8 ++++++++
3 files changed, 12 insertions(+)

diff --git a/drivers/isdn/mISDN/core.c b/drivers/isdn/mISDN/core.c
index da30c5c..a7050c3 100644
--- a/drivers/isdn/mISDN/core.c
+++ b/drivers/isdn/mISDN/core.c
@@ -21,10 +21,13 @@
#include "core.h"

static u_int debug;
+u_int misdn_permitted_gid;

MODULE_AUTHOR("Karsten Keil");
MODULE_LICENSE("GPL");
module_param(debug, uint, S_IRUGO | S_IWUSR);
+module_param_named(gid, misdn_permitted_gid, uint, 0);
+MODULE_PARM_DESC(gid, "Unix group for accessing misdn socket (default 0)");

static u64 device_ids;
#define MAX_DEVICE_ID 63
diff --git a/drivers/isdn/mISDN/core.h b/drivers/isdn/mISDN/core.h
index 52695bb..6ea673f 100644
--- a/drivers/isdn/mISDN/core.h
+++ b/drivers/isdn/mISDN/core.h
@@ -17,6 +17,7 @@

extern struct mISDNdevice *get_mdevice(u_int);
extern int get_mdevice_count(void);
+extern u_int misdn_permitted_gid;

/* stack status flag */
#define mISDN_STACK_ACTION_MASK 0x0000ffff
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index e47dcb9..71f4986 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -612,6 +612,10 @@ data_sock_create(struct net *net, struct socket *sock, int protocol)
{
struct sock *sk;

+ if(!capable(CAP_SYS_ADMIN) && (misdn_permitted_gid != current_gid())
+ && (!in_group_p(misdn_permitted_gid)))
+ return -EPERM;
+
if (sock->type != SOCK_DGRAM)
return -ESOCKTNOSUPPORT;

@@ -694,6 +698,10 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
case IMSETDEVNAME:
{
struct mISDN_devrename dn;
+ if(!capable(CAP_SYS_ADMIN)
+ && (misdn_permitted_gid != current_gid())
+ && (!in_group_p(misdn_permitted_gid)))
+ return -EPERM;
if (copy_from_user(&dn, (void __user *)arg,
sizeof(dn))) {
err = -EFAULT;
--
1.8.2.1

2013-05-07 14:21:32

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

From: Jeff Mahoney <[email protected]>

The chipidea driver currently has needless ifneq rules in the makefile
for things that should be config options. This can be problematic,
especially in the IMX case, since the OF_DEVICE dependency will be met
on powerpc systems - which don't actually support the hardware via that
method.

This patch adds _PCI and _IMX config options to allow the user to
select whether to build the modules.

Signed-off-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
---
drivers/usb/chipidea/Kconfig | 11 +++++++++++
drivers/usb/chipidea/Makefile | 11 ++---------
2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index 608a2ae..1ac9bc2 100644
--- a/drivers/usb/chipidea/Kconfig
+++ b/drivers/usb/chipidea/Kconfig
@@ -31,4 +31,15 @@ config USB_CHIPIDEA_DEBUG
help
Say Y here to enable debugging output of the ChipIdea driver.

+config USB_CHIPIDEA_PCI
+ bool "ChipIdea PCI support"
+ depends on PCI
+ help
+ This option enables ChipIdea support on PCI.
+
+config USB_CHIPIDEA_IMX
+ bool "ChipIdea IMX support"
+ depends on OF_DEVICE
+ help
+ This option enables ChipIdea support on IMX.
endif
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 4ab83e9..a97d103 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -10,12 +10,5 @@ ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG) += debug.o
# Glue/Bridge layers go here

obj-$(CONFIG_USB_CHIPIDEA) += ci13xxx_msm.o
-
-# PCI doesn't provide stubs, need to check
-ifneq ($(CONFIG_PCI),)
- obj-$(CONFIG_USB_CHIPIDEA) += ci13xxx_pci.o
-endif
-
-ifneq ($(CONFIG_OF_DEVICE),)
- obj-$(CONFIG_USB_CHIPIDEA) += ci13xxx_imx.o usbmisc_imx.o
-endif
+obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci13xxx_pci.o
+obj-$(CONFIG_USB_CHIPIDEA_IMX) += ci13xxx_imx.o usbmisc_imx.o
--
1.8.2.1

2013-05-07 14:27:58

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 09/15] Make selection of 'readdir-plus' adapt to usage patterns.

On Tue, 2013-05-07 at 16:18 +0200, Jiri Slaby wrote:
> From: NeilBrown <[email protected]>
>
> While the use of READDIRPLUS is significantly more efficient than
> READDIR followed by many GETATTR calls, it is still less efficient
> than just READDIR if the attributes are not required.
>
> We can get a hint as to whether the application requires attr information
> by looking at whether any ->getattr calls are made between
> ->readdir calls.
> If there are any, then getting the attributes seems to be worth while.
>
> This patch tracks whether there have been recent getattr calls on
> children of a directory and uses that information to selectively
> disable READDIRPLUS on that directory.
>
> The first 'readdir' call is always served using READDIRPLUS.
> Subsequent calls only use READDIRPLUS if there was a getattr on a child
> in the mean time.
>
> The locking of ->d_parent access needs to be reviewed.
> As the bit is simply a hint, it isn't critical that it is set
> on the "correct" parent if a rename is happening, but it is
> critical that the 'set' doesn't set a bit in something that
> isn't even an inode any more.
>
> Acked-by: NeilBrown <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Trond Myklebust <[email protected]>
> Cc: [email protected]
> ---

Why am I being Cced on this?

You have looked at commit d69ee9b85541a69a1092f5da675bd23256dc62af (NFS:
Adapt readdirplus to application usage patterns) which was upstreamed in
Linux 3.5, right?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-05-07 14:32:33

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 09/15] Make selection of 'readdir-plus' adapt to usage patterns.

On 05/07/2013 04:27 PM, Myklebust, Trond wrote:
> On Tue, 2013-05-07 at 16:18 +0200, Jiri Slaby wrote:
>> From: NeilBrown <[email protected]>
>>
>> While the use of READDIRPLUS is significantly more efficient than
>> READDIR followed by many GETATTR calls, it is still less efficient
>> than just READDIR if the attributes are not required.
>>
>> We can get a hint as to whether the application requires attr information
>> by looking at whether any ->getattr calls are made between
>> ->readdir calls.
>> If there are any, then getting the attributes seems to be worth while.
>>
>> This patch tracks whether there have been recent getattr calls on
>> children of a directory and uses that information to selectively
>> disable READDIRPLUS on that directory.
>>
>> The first 'readdir' call is always served using READDIRPLUS.
>> Subsequent calls only use READDIRPLUS if there was a getattr on a child
>> in the mean time.
>>
>> The locking of ->d_parent access needs to be reviewed.
>> As the bit is simply a hint, it isn't critical that it is set
>> on the "correct" parent if a rename is happening, but it is
>> critical that the 'set' doesn't set a bit in something that
>> isn't even an inode any more.
>>
>> Acked-by: NeilBrown <[email protected]>
>> Signed-off-by: Neil Brown <[email protected]>
>> Signed-off-by: Jiri Slaby <[email protected]>
>> Cc: Trond Myklebust <[email protected]>
>> Cc: [email protected]
>> ---
>
> Why am I being Cced on this?

Because we have the patch still in _3.9_. Dropped now in favor of the
commit below.

> You have looked at commit d69ee9b85541a69a1092f5da675bd23256dc62af (NFS:
> Adapt readdirplus to application usage patterns) which was upstreamed in
> Linux 3.5, right?

Thanks for letting me know.

--
js
suse labs

2013-05-07 14:48:43

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 01/15] Char: lp, protect LPGETSTATUS with port_mutex

On Tuesday 07 May 2013, Jiri Slaby wrote:
> From: "[email protected]" <[email protected]>
>
> The patch fixes a problem in the lp driver that can cause oopses as
> follows:
> process A: calls lp_write, which in turn calls
> parport_ieee1284_write_compat, and that invokes
> parport_wait_peripheral
> process B: meanwhile does an ioctl(LPGETSTATUS), which call
> lp_release_parport when done. This function will set
> physport->cad = NULL.
> process A: parport_wait_peripheral tries to dereference
> physport->cad and dies
>
> So, protect that code with the port_mutex in order to protect against
> simultaneous calls to lp_read/lp_write.
>
> Similar protection is probably required for ioctl(LPRESET)...
>
> This patch was done by IBM a while back and we (at suse) have that
> since at least 2004 in our repos. Let's make it upstream.

Hmm, it seems the driver has changed a bit since 2004, at least when
I added the lp_mutex to lp_open()/lp_ioctl(). It's probably worth
taking a look at the bigger picture now, to combine lp_mutex with
lp_table[minor].port_mutex. I don't see any reason why we can't always
use the per-device mutex. The only shared variable is the lp_count
number, and that is not protected under lp_mutex today, and presumably
not updated at run time either.

Arnd

2013-05-07 15:40:46

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 01/15] Char: lp, protect LPGETSTATUS with port_mutex

On 05/07/2013 04:48 PM, Arnd Bergmann wrote:
> On Tuesday 07 May 2013, Jiri Slaby wrote:
>> From: "[email protected]" <[email protected]>
>>
>> The patch fixes a problem in the lp driver that can cause oopses as
>> follows:
>> process A: calls lp_write, which in turn calls
>> parport_ieee1284_write_compat, and that invokes
>> parport_wait_peripheral
>> process B: meanwhile does an ioctl(LPGETSTATUS), which call
>> lp_release_parport when done. This function will set
>> physport->cad = NULL.
>> process A: parport_wait_peripheral tries to dereference
>> physport->cad and dies
>>
>> So, protect that code with the port_mutex in order to protect against
>> simultaneous calls to lp_read/lp_write.
>>
>> Similar protection is probably required for ioctl(LPRESET)...
>>
>> This patch was done by IBM a while back and we (at suse) have that
>> since at least 2004 in our repos. Let's make it upstream.
>
> Hmm, it seems the driver has changed a bit since 2004, at least when
> I added the lp_mutex to lp_open()/lp_ioctl(). It's probably worth
> taking a look at the bigger picture now, to combine lp_mutex with
> lp_table[minor].port_mutex. I don't see any reason why we can't always
> use the per-device mutex.

Yeah, it looks sensible to me too to get rid of the lp_mutex, another
BKL left-over. However I don't have the hardware, the patch I attached
was taken from our tree and tested, at least some time ago. Patches to
clean that mess up welcome.

thanks,
--
js
suse labs

2013-05-07 19:03:52

by Viacheslav Dubeyko

[permalink] [raw]
Subject: Re: [PATCH 07/15] hfs: avoid crash in hfs_bnode_create

On May 7, 2013, at 6:18 PM, Jiri Slaby wrote:

> From: Jeff Mahoney <[email protected]>
>
> Commit 634725a92938b0f282b17cec0b007dca77adebd2 removed the BUG_ON in
> hfs_bnode_create in hfsplus. This patch removes it from the hfs
> version and avoids an fsfuzzer crash.
>
> Signed-off-by: Jeff Mahoney <[email protected]>
> Acked-by: Jeff Mahoney <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: [email protected]
> Cc: Andrew Morton <[email protected]>
> ---
> fs/hfs/bnode.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index f3b1a15..6d435c2 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -415,7 +415,11 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
> spin_lock(&tree->hash_lock);
> node = hfs_bnode_findhash(tree, num);
> spin_unlock(&tree->hash_lock);
> - BUG_ON(node);
> + if (node) {
> + printk(KERN_CRIT "new node %u already hashed?\n", num);

The error/debug subsystem of HFS/HFS+ was reworked by Joe Perches, recently. Please, change printk() on pr_crit().

Thanks,
Vyacheslav Dubeyko.

> + WARN_ON(1);
> + return node;
> + }
> node = __hfs_bnode_create(tree, num);
> if (!node)
> return ERR_PTR(-ENOMEM);
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-05-07 19:29:51

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

On Tue, May 07, 2013 at 04:18:23PM +0200, Jiri Slaby wrote:
> From: Jeff Mahoney <[email protected]>
>
> The PCH EG20T is only compatible with Intel Atom processors so it
> should depend on x86.

This patch has been submitted before,

https://patchwork.kernel.org/patch/2069071/

and at that time the reaction was that it is good to have drivers
cross-compiled, if only for code quality reasons.

Thanks,
Richard

2013-05-08 02:03:39

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH 05/15] connection tracking helper for SLP

On Tue, May 07, 2013 at 04:18:13PM +0200, Jiri Slaby wrote:
> From: Jiri Bohac <[email protected]>
>
> A simple connection tracking helper for SLP. Marks replies to a
> SLP broadcast query as ESTABLISHED to allow them to pass through the
> firewall.
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: "David S. Miller" <[email protected]>
> Cc: Patrick McHardy <[email protected]>
> Cc: Pablo Neira Ayuso <[email protected]>
> ---
> net/netfilter/Kconfig | 15 +++++
> net/netfilter/Makefile | 1 +
> net/netfilter/nf_conntrack_slp.c | 131 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 147 insertions(+)
> create mode 100644 net/netfilter/nf_conntrack_slp.c
>
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 56d22ca..ec61b30 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -320,6 +320,21 @@ config NF_CONNTRACK_TFTP
>
> To compile it as a module, choose M here. If unsure, say N.
>
> +config NF_CONNTRACK_SLP
> + tristate "SLP protocol support"
> + depends on NF_CONNTRACK
> + depends on NETFILTER_ADVANCED
> + help
> + SLP queries are sometimes sent as broadcast messages from an
> + unprivileged port and responded to with unicast messages to the
> + same port. This make them hard to firewall properly because connection
> + tracking doesn't deal with broadcasts. This helper tracks locally
> + originating broadcast SLP queries and the corresponding
> + responses. It relies on correct IP address configuration, specifically
> + netmask and broadcast address.

We have the user-space helper infrastructure in the conntrack-tools,
this helper has to go there.

2013-05-08 09:04:58

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

Jiri Slaby <[email protected]> writes:

> From: Jeff Mahoney <[email protected]>
>
> The chipidea driver currently has needless ifneq rules in the makefile
> for things that should be config options.

Please elaborate on the "should be" part.

> This can be problematic,
> especially in the IMX case, since the OF_DEVICE dependency will be met
> on powerpc systems - which don't actually support the hardware via that
> method.

That's all right, but these things should still compile on powerpc and
get more compilation testing like that. On the other hand, if the
compilation does break, we're probably looking at a bug in ci13xxx_imx,
which needs fixing.

> This patch adds _PCI and _IMX config options to allow the user to
> select whether to build the modules.

I would really like to avoid unnecessary config options in the chipidea
driver, so my question is: is there a real bug or compilation breakage
that is triggered in the current state of things?

> +config USB_CHIPIDEA_PCI
> + bool "ChipIdea PCI support"
> + depends on PCI
> + help
> + This option enables ChipIdea support on PCI.

I totally don't understand this: we have CONFIG_USB_CHIPIDEA and
CONFIG_PCI, which already enable chipidea support on PCI. This helps in
the case when you have both options enabled, but still don't want the
ci13xxx_pci module to be built, but it doesn't justify an extra option.

Regards,
--
Alex

2013-05-08 15:25:42

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v2 07/14] hfs: avoid crash in hfs_bnode_create

From: Jeff Mahoney <[email protected]>

Commit 634725a92938b0f282b17cec0b007dca77adebd2 removed the BUG_ON in
hfs_bnode_create in hfsplus. This patch removes it from the hfs
version and avoids an fsfuzzer crash.

[v2]
- use pr_crit (per Vyacheslav)

Signed-off-by: Jeff Mahoney <[email protected]>
Acked-by: Jeff Mahoney <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Cc: Vyacheslav Dubeyko <[email protected]>
---
fs/hfs/bnode.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index f3b1a15..d3fa6bd 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -415,7 +415,11 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
spin_lock(&tree->hash_lock);
node = hfs_bnode_findhash(tree, num);
spin_unlock(&tree->hash_lock);
- BUG_ON(node);
+ if (node) {
+ pr_crit("new node %u already hashed?\n", num);
+ WARN_ON(1);
+ return node;
+ }
node = __hfs_bnode_create(tree, num);
if (!node)
return ERR_PTR(-ENOMEM);
--
1.8.2.2

2013-05-14 14:19:44

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

On 05/07/2013 09:29 PM, Richard Cochran wrote:
> On Tue, May 07, 2013 at 04:18:23PM +0200, Jiri Slaby wrote:
>> From: Jeff Mahoney <[email protected]>
>>
>> The PCH EG20T is only compatible with Intel Atom processors so it
>> should depend on x86.
>
> This patch has been submitted before,
>
> https://patchwork.kernel.org/patch/2069071/
>
> and at that time the reaction was that it is good to have drivers
> cross-compiled, if only for code quality reasons.

Hmm, then it depends whether the kernel is for users or for developers.
I, as a user, do not really want to compile drivers with allmodconfig
which I have no way to load/use.

And allmodconfig is basically what we, users (SUSE distributors in this
case) do. So having this driver being built makes our life harder (in
the meaning we have to have specific rules about disabling unwanted
drivers from configs).

And, developers should have a testbed where they build 32bit configs.
And we actually have that for -next AFAIU.

thanks,
--
js
suse labs

2013-05-14 18:12:39

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

On Tue, May 14, 2013 at 04:20:09PM +0200, Jiri Slaby wrote:
> On 05/07/2013 09:29 PM, Richard Cochran wrote:
> > On Tue, May 07, 2013 at 04:18:23PM +0200, Jiri Slaby wrote:
> >> From: Jeff Mahoney <[email protected]>
> >>
> >> The PCH EG20T is only compatible with Intel Atom processors so it
> >> should depend on x86.
> >
> > This patch has been submitted before,
> >
> > https://patchwork.kernel.org/patch/2069071/
> >
> > and at that time the reaction was that it is good to have drivers
> > cross-compiled, if only for code quality reasons.
>
> Hmm, then it depends whether the kernel is for users or for developers.
> I, as a user, do not really want to compile drivers with allmodconfig
> which I have no way to load/use.
>
> And allmodconfig is basically what we, users (SUSE distributors in this
> case) do. So having this driver being built makes our life harder (in
> the meaning we have to have specific rules about disabling unwanted
> drivers from configs).

Wearing my Debian hat, I agree with this. Perhaps we could define a
CONFIG_BUILD_TEST symbol for people who want to do that, and then
make drivers for hardware that's only found in x86 systems (for
example) depend on X86 || BUILD_TEST.

Ben.

> And, developers should have a testbed where they build 32bit configs.
> And we actually have that for -next AFAIU.

--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus

2013-05-15 15:58:41

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

On 05/08/2013 11:07 AM, Alexander Shishkin wrote:
> Jiri Slaby <[email protected]> writes:
>
>> From: Jeff Mahoney <[email protected]>
>>
>> The chipidea driver currently has needless ifneq rules in the makefile
>> for things that should be config options.
>
> Please elaborate on the "should be" part.
>
>> This can be problematic,
>> especially in the IMX case, since the OF_DEVICE dependency will be met
>> on powerpc systems - which don't actually support the hardware via that
>> method.
>
> That's all right, but these things should still compile on powerpc and
> get more compilation testing like that. On the other hand, if the
> compilation does break, we're probably looking at a bug in ci13xxx_imx,
> which needs fixing.
>
>> This patch adds _PCI and _IMX config options to allow the user to
>> select whether to build the modules.
>
> I would really like to avoid unnecessary config options in the chipidea
> driver, so my question is: is there a real bug or compilation breakage
> that is triggered in the current state of things?
>
>> +config USB_CHIPIDEA_PCI
>> + bool "ChipIdea PCI support"
>> + depends on PCI
>> + help
>> + This option enables ChipIdea support on PCI.
>
> I totally don't understand this: we have CONFIG_USB_CHIPIDEA and
> CONFIG_PCI, which already enable chipidea support on PCI. This helps in
> the case when you have both options enabled, but still don't want the
> ci13xxx_pci module to be built, but it doesn't justify an extra option.

Hi, the whole point of the patch is that there is no reason in building
the imx part of the driver on powerpc, because ppc will never provide
that of_device. So we are adding that option and disable that in suse
completely by this patch plus a config option.

The PCI case is not necessarily needed, but follows the IMX case.

thanks,
--
js
suse labs

2013-05-16 09:36:36

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

Jiri Slaby <[email protected]> writes:

> On 05/08/2013 11:07 AM, Alexander Shishkin wrote:
>> Jiri Slaby <[email protected]> writes:
>>
>>> From: Jeff Mahoney <[email protected]>
>>>
>>> The chipidea driver currently has needless ifneq rules in the makefile
>>> for things that should be config options.
>>
>> Please elaborate on the "should be" part.
>>
>>> This can be problematic,
>>> especially in the IMX case, since the OF_DEVICE dependency will be met
>>> on powerpc systems - which don't actually support the hardware via that
>>> method.
>>
>> That's all right, but these things should still compile on powerpc and
>> get more compilation testing like that. On the other hand, if the
>> compilation does break, we're probably looking at a bug in ci13xxx_imx,
>> which needs fixing.
>>
>>> This patch adds _PCI and _IMX config options to allow the user to
>>> select whether to build the modules.
>>
>> I would really like to avoid unnecessary config options in the chipidea
>> driver, so my question is: is there a real bug or compilation breakage
>> that is triggered in the current state of things?
>>
>>> +config USB_CHIPIDEA_PCI
>>> + bool "ChipIdea PCI support"
>>> + depends on PCI
>>> + help
>>> + This option enables ChipIdea support on PCI.
>>
>> I totally don't understand this: we have CONFIG_USB_CHIPIDEA and
>> CONFIG_PCI, which already enable chipidea support on PCI. This helps in
>> the case when you have both options enabled, but still don't want the
>> ci13xxx_pci module to be built, but it doesn't justify an extra option.
>
> Hi, the whole point of the patch is that there is no reason in building
> the imx part of the driver on powerpc, because ppc will never provide
> that of_device. So we are adding that option and disable that in suse
> completely by this patch plus a config option.

The benefit from compiling it on non-arm (or non-imx) architectures for
me is compilation testing. We don't have a whole lot of architectures
with CONFIG_OF so it's nice to give it a stretch.

If you really want to save space in your rpm package, I would suggest
you add a condition to Makefile (ifeq ($(ARCH),arm) or something like
that) instead of a Kconfig option. I would prefer at least to avoid the
unnecessary kconfig option creep.

> The PCI case is not necessarily needed, but follows the IMX case.

But then, if you want to disable both IMX and PCI you're better off
disabling the whole chipidea instead.

Thanks,
--
Alex

2013-05-20 22:31:31

by Jacob Keller

[permalink] [raw]
Subject: RE: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

> -----Original Message-----
> From: [email protected] [mailto:netdev-
> [email protected]] On Behalf Of Ben Hutchings
> Sent: Tuesday, May 14, 2013 11:13 AM
> To: Jiri Slaby
> Cc: Richard Cochran; Jiri Slaby; [email protected]; Jeff
> Mahoney; [email protected]
> Subject: Re: [PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86
>
> On Tue, May 14, 2013 at 04:20:09PM +0200, Jiri Slaby wrote:
> > On 05/07/2013 09:29 PM, Richard Cochran wrote:
> > > On Tue, May 07, 2013 at 04:18:23PM +0200, Jiri Slaby wrote:
> > >> From: Jeff Mahoney <[email protected]>
> > >>
> > >> The PCH EG20T is only compatible with Intel Atom processors so it
> > >> should depend on x86.
> > >
> > > This patch has been submitted before,
> > >
> > > https://patchwork.kernel.org/patch/2069071/
> > >
> > > and at that time the reaction was that it is good to have drivers
> > > cross-compiled, if only for code quality reasons.
> >
> > Hmm, then it depends whether the kernel is for users or for
> developers.
> > I, as a user, do not really want to compile drivers with allmodconfig
> > which I have no way to load/use.
> >
> > And allmodconfig is basically what we, users (SUSE distributors in this
> > case) do. So having this driver being built makes our life harder (in
> > the meaning we have to have specific rules about disabling unwanted
> > drivers from configs).
>
> Wearing my Debian hat, I agree with this. Perhaps we could define a
> CONFIG_BUILD_TEST symbol for people who want to do that, and then
> make drivers for hardware that's only found in x86 systems (for
> example) depend on X86 || BUILD_TEST.
>
> Ben.
>

I agree with this.

- Jake Keller

> > And, developers should have a testbed where they build 32bit configs.
> > And we actually have that for -next AFAIU.
> > --
> Ben Hutchings
> We get into the habit of living before acquiring the habit of thinking.
> - Albert Camus
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-05-22 08:50:48

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

On 05/16/2013 11:36 AM, Alexander Shishkin wrote:
> The benefit from compiling it on non-arm (or non-imx) architectures for
> me is compilation testing. We don't have a whole lot of architectures
> with CONFIG_OF so it's nice to give it a stretch.

Yes, the newly added COMPILE_TEST config option will do the job. Stay
tuned...

> If you really want to save space in your rpm package, I would suggest
> you add a condition to Makefile (ifeq ($(ARCH),arm) or something like
> that) instead of a Kconfig option. I would prefer at least to avoid the
> unnecessary kconfig option creep.

Uhuh, why should *users* patch the kernel prior building it with
allmodconfig? We do not want to carry *any* patches with our kernels.

thanks,
--
js
suse labs

2013-05-22 11:10:59

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH 13/15] chipidea: Allow user to select PCI/IMX options

Jiri Slaby <[email protected]> writes:

> On 05/16/2013 11:36 AM, Alexander Shishkin wrote:
>> The benefit from compiling it on non-arm (or non-imx) architectures for
>> me is compilation testing. We don't have a whole lot of architectures
>> with CONFIG_OF so it's nice to give it a stretch.
>
> Yes, the newly added COMPILE_TEST config option will do the job. Stay
> tuned...

Seems to do the trick.

>> If you really want to save space in your rpm package, I would suggest
>> you add a condition to Makefile (ifeq ($(ARCH),arm) or something like
>> that) instead of a Kconfig option. I would prefer at least to avoid the
>> unnecessary kconfig option creep.
>
> Uhuh, why should *users* patch the kernel prior building it with
> allmodconfig? We do not want to carry *any* patches with our kernels.

I meant that instead of adding more options we could use the existing
ones in a Makefile, not that you do that in distro kernel only. But
COMPILE_TEST seems to fit the bill quite nicely anyway.

Regards,
--
Alex