lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 25 Feb 2008 18:23:11 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	David Miller <davem@...emloft.net>
Cc:	kaber@...sh.net, joe@...ches.com, harvey.harrison@...il.com,
	netdev@...r.kernel.org
Subject: Re: New sparse warning in net/mac80211/debugfs_sta.c


On Mon, 2008-02-25 at 10:53 +0100, Johannes Berg wrote:
> On Sat, 2008-02-23 at 20:02 -0800, David Miller wrote:
> > From: Patrick McHardy <kaber@...sh.net>
> > Date: Thu, 21 Feb 2008 19:00:03 +0100
> > 
> > > And adds back the overhead of two completely unnecessary
> > > function calls to the VLAN fastpath. How about just
> > > stopping this idiocy and reverting the appropriate patches
> > > to bring back MAC_FMT and use it where appropriate?
> > 
> > Agreed, I'll do that.
> 
> Maybe we should just add a new printf modifier like %M for MAC
> addresses? Then we could use sprintf, snprintf, printk and whatever we
> please without any of the macro stuff...

I have something like this in mind. Might even be faster than using the
MAC_FMT/MAC_ARG stuff because it's done in a single loop rather than
invoking the hex digit printing 6 times.


From: Johannes Berg <johannes@...solutions.net>
Subject: MAC printing with %M/%m

We've been through two ways to print MAC addresses to the kernel
logs/buffers/whatever.

Until recently, we had

#define MAC_FMT		"%.2x:%.2x:%.2x:..."
#define MAC_ARG(m)	(m)[0], (m)[1], (m)[2], ...

printk("bla bla " MAC_FMT "\n", MAC_ARG(mac));

This is fairly ugly and was found to lead to quite long strings
embedded in the kernel, all the %.2x stuff.


Recently, we changed to using print_mac():

DECLARE_MAC_BUF(mbuf);
printk("bla bla %s\n", print_mac(mbuf, mac));

This was, however, found to force the compiler to emit print_mac()
function calls in fast paths even when debugging was turned off.


Here's my take on it, putting it right into the vsnprintf code.
It allows you to write

printk("bla bla %m\n", mac);

without any further function calls or local variables.

The only problem I see with using 'm' and 'M' is that 'm' is already
defined by glibc to print strerror(errno).

This patch doesn't contain any conversion yet but that could happen
gradually, starting with the fast paths.

I've tested this code in userspace with a number of MAC addresses
and various output buffer limits.

Field length specifiers are allowed and treated as if the already
printed MAC address was given to a %s specifier, ie.
	%-20m	is like %-20s
etc.

Signed-off-by: Johannes Berg <johannes@...solutions.net>
---
 lib/vsprintf.c |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

--- everything.orig/lib/vsprintf.c	2008-02-25 17:31:56.000000000 +0100
+++ everything/lib/vsprintf.c	2008-02-25 17:57:34.000000000 +0100
@@ -366,11 +366,11 @@ static noinline char* put_dec(char *buf,
 #define SMALL	32		/* Must be 32 == 0x20 */
 #define SPECIAL	64		/* 0x */
 
+/* we are called with base 8, 10 or 16, only, thus don't need "G..."  */
+static const char hexdigits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
+
 static char *number(char *buf, char *end, unsigned long long num, int base, int size, int precision, int type)
 {
-	/* we are called with base 8, 10 or 16, only, thus don't need "G..."  */
-	static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
-
 	char tmp[66];
 	char sign;
 	char locase;
@@ -408,7 +408,7 @@ static char *number(char *buf, char *end
 		tmp[i++] = '0';
 	/* Generic code, for any base:
 	else do {
-		tmp[i++] = (digits[do_div(num,base)] | locase);
+		tmp[i++] = (hexdigits[do_div(num,base)] | locase);
 	} while (num != 0);
 	*/
 	else if (base != 10) { /* 8 or 16 */
@@ -416,7 +416,7 @@ static char *number(char *buf, char *end
 		int shift = 3;
 		if (base == 16) shift = 4;
 		do {
-			tmp[i++] = (digits[((unsigned char)num) & mask] | locase);
+			tmp[i++] = (hexdigits[((unsigned char)num) & mask] | locase);
 			num >>= shift;
 		} while (num);
 	} else { /* base 10 */
@@ -621,6 +621,43 @@ int vsnprintf(char *buf, size_t size, co
 				}
 				continue;
 
+
+			case 'm':
+				flags |= SMALL;
+			case 'M':
+				s = va_arg(args, unsigned char *);
+				len = 17;
+
+				if (!(flags & LEFT)) {
+					while (len < field_width--) {
+						if (str < end)
+							*str = ' ';
+						++str;
+					}
+				}
+				for (i = 0; i < len; i++) {
+					if (str < end) {
+						if ((i % 3) == 0)
+							*str = hexdigits[(*(unsigned char *)s) >> 4];
+						else if ((i % 3) == 1)
+							*str = hexdigits[(*s) & 0xF];
+						else {
+							*str = ':';
+							s++;
+						}
+						if (flags & SMALL)
+							*str = TOLOWER(*str);
+					}
+					++str;
+				}
+				while (len < field_width--) {
+					if (str < end)
+						*str = ' ';
+					++str;
+				}
+				continue;
+
+
 			case 's':
 				s = va_arg(args, char *);
 				if ((unsigned long)s < PAGE_SIZE)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ