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

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

[Jiri Slaby]

From: "salina@us.ibm.com" <salina@us.ibm.com>

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: okir@suse.de
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <isdn4linux@koppen.de>

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

Acked-by: Jeff Mahoney <jeffm@suse.com>
Cc: Patrick Koppen <isdn4linux@koppen.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
---
 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

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

[Jiri Slaby]

From: Olaf Hering <ohering@suse.com>

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 <jeffm@suse.com>
Signed-off-by: Olaf Hering <ohering@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
---
 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

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

[Jiri Slaby]

From: Takashi Iwai <tiwai@suse.de>

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: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
 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

[PATCH 05/15] connection tracking helper for SLP

[Jiri Slaby]

From: Jiri Bohac <jbohac@suse.cz>

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 <jbohac@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: netfilter-devel@vger.kernel.org
Cc: netfilter@vger.kernel.org
Cc: coreteam@netfilter.org
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
---
 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 <jbohac@suse.cz>
+ *      (c) 2005 Patrick McHardy <kaber@trash.net>
+ *
+ *      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 <jbohac@suse.cz>");
+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

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

[Pablo Neira Ayuso]

On Tue, May 07, 2013 at 04:18:13PM +0200, Jiri Slaby wrote:
> From: Jiri Bohac <jbohac@suse.cz>
> 
> 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 <jbohac@suse.cz>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: netfilter-devel@vger.kernel.org
> Cc: netfilter@vger.kernel.org
> Cc: coreteam@netfilter.org
> Cc: netdev@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  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.

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

 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 <mpost@novell.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netfilter-devel@vger.kernel.org
Cc: netfilter@vger.kernel.org
Cc: coreteam@netfilter.org
Cc: netdev@vger.kernel.org
---
 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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <jeffm@suse.com>
Acked-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: linux-fsdevel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 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

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

[Vyacheslav Dubeyko]

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

> From: Jeff Mahoney <jeffm@suse.com>
> 
> 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 <jeffm@suse.com>
> Acked-by: Jeff Mahoney <jeffm@suse.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
> 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 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/

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <jeffm@suse.com>
Acked-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: linux-fsdevel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
---
 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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: linux-fsdevel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: reiserfs-devel@vger.kernel.org
---
 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

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

[Jiri Slaby]

From: NeilBrown <neilb@suse.de>

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 <neilb@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: linux-nfs@vger.kernel.org
---
 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

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

[Myklebust, Trond]

On Tue, 2013-05-07 at 16:18 +0200, Jiri Slaby wrote:
> From: NeilBrown <neilb@suse.de>
> 
> 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 <neilb@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: linux-nfs@vger.kernel.org
> ---

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
Trond.Myklebust@netapp.com
www.netapp.com

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

[Jiri Slaby]

On 05/07/2013 04:27 PM, Myklebust, Trond wrote:
> On Tue, 2013-05-07 at 16:18 +0200, Jiri Slaby wrote:
>> From: NeilBrown <neilb@suse.de>
>>
>> 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 <neilb@suse.de>
>> Signed-off-by: Neil Brown <neilb@suse.de>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
>> Cc: linux-nfs@vger.kernel.org
>> ---
> 
> 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

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

[Jiri Slaby]

From: Vasiliy Kulikov <segooon@gmail.com>

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

Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 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 <trenn@suse.de>");
@@ -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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
 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

[PATCH 12/15] geodefb: Depend on X86_32

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-geode@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
---
 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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
---
 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

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

[Alexander Shishkin]

Jiri Slaby <jslaby@suse.cz> writes:

> From: Jeff Mahoney <jeffm@suse.com>
>
> 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

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

[Jiri Slaby]

On 05/08/2013 11:07 AM, Alexander Shishkin wrote:
> Jiri Slaby <jslaby@suse.cz> writes:
> 
>> From: Jeff Mahoney <jeffm@suse.com>
>>
>> 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

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

[Alexander Shishkin]

Jiri Slaby <jslaby@suse.cz> writes:

> On 05/08/2013 11:07 AM, Alexander Shishkin wrote:
>> Jiri Slaby <jslaby@suse.cz> writes:
>> 
>>> From: Jeff Mahoney <jeffm@suse.com>
>>>
>>> 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

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

[Jiri Slaby]

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

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

[Alexander Shishkin]

Jiri Slaby <jslaby@suse.cz> 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

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

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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 <rostedt@goodmis.org>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org
---
 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

[PATCH 15/15] ptp: PTP_1588_CLOCK_PCH depends on x86

[Jiri Slaby]

From: Jeff Mahoney <jeffm@suse.com>

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

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: netdev@vger.kernel.org
---
 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

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

[Richard Cochran]

On Tue, May 07, 2013 at 04:18:23PM +0200, Jiri Slaby wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> 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

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

[Jiri Slaby]

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 <jeffm@suse.com>
>>
>> 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

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

[Ben Hutchings]

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 <jeffm@suse.com>
> >>
> >> 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

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

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Ben Hutchings
> Sent: Tuesday, May 14, 2013 11:13 AM
> To: Jiri Slaby
> Cc: Richard Cochran; Jiri Slaby; linux-kernel@vger.kernel.org; Jeff
> Mahoney; netdev@vger.kernel.org
> 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 <jeffm@suse.com>
> > >>
> > >> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Arnd Bergmann]

On Tuesday 07 May 2013, Jiri Slaby wrote:
> From: "salina@us.ibm.com" <salina@us.ibm.com>
> 
> 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

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

[Jiri Slaby]

On 05/07/2013 04:48 PM, Arnd Bergmann wrote:
> On Tuesday 07 May 2013, Jiri Slaby wrote:
>> From: "salina@us.ibm.com" <salina@us.ibm.com>
>>
>> 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