[PATCH v2 1/2] vsprintf: Fix potential unaligned access

Andy Shevchenko

author reviewer

2022-01-24 14:42 UTC #0 lore raw

PATCH v2 1/2 vsprintf: Fix potential unaligned access

The %p4cc specifier in some cases might get an unaligned pointer. Due to this we need to make copy to local variable once to avoid potential crashes on some architectures due to improper access.

Fixes af612e43de6d ("lib/vsprintf: Add support for printing V4L2 and DRM fourccs")

Cc Sakari Ailus <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

Reviewed-by Petr Mladek <[email protected]>

---
v2: added Petr's tag

 lib/vsprintf.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

lib/vsprintf.c +7 -5

index 61528094ec87..4e8f3e9acb99 100644
@@ -49,6 +49,7 @@
 
 #include <asm/page.h>		/* for PAGE_SIZE */
 #include <asm/byteorder.h>	/* cpu_to_le16 */
+#include <asm/unaligned.h>
 
 #include <linux/string_helpers.h>
 #include "kstrtox.h"

@@ -1762,7 +1763,7 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 	char output[sizeof("0123 little-endian (0x01234567)")];
 	char *p = output;
 	unsigned int i;
-	u32 val;
+	u32 orig, val;
 
 	if (fmt[1] != 'c' || fmt[2] != 'c')
 		return error_string(buf, end, "(%p4?)", spec);

@@ -1770,21 +1771,22 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 	if (check_pointer(&buf, end, fourcc, spec))
 		return buf;
 
-	val = *fourcc & ~BIT(31);
+	orig = get_unaligned(fourcc);
+	val = orig & ~BIT(31);
 
-	for (i = 0; i < sizeof(*fourcc); i++) {
+	for (i = 0; i < sizeof(u32); i++) {
 		unsigned char c = val >> (i * 8);
 
 		/* Print non-control ASCII characters as-is, dot otherwise */
 		*p++ = isascii(c) && isprint(c) ? c : '.';
 	}
 
-	strcpy(p, *fourcc & BIT(31) ? " big-endian" : " little-endian");
+	strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian");
 	p += strlen(p);
 
 	*p++ = ' ';
 	*p++ = '(';
-	p = special_hex_number(p, output + sizeof(output) - 2, *fourcc, sizeof(u32));
+	p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
 	*p++ = ')';
 	*p = '\0';

Signature · 1 line

2.34.1

Andy Shevchenko

author reviewer

2022-01-24 14:42 UTC #1 lore raw

PATCH v2 2/2 vsprintf: Move space out of string literals in fourcc_string()

The literals "big-endian" and "little-endian" may be potentially occurred in other places. Dropping space allows linker to "compress" them by using only a single copy.

Rasmus suggested, while at it, replacing strcpy() + strlen() by p = stpcpy(), which is done here as well.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Sakari Ailus <[email protected]>

---
v2: added Sakari's tag, replaced APIs as mentioned in the commit (Rasmus)

 lib/vsprintf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

lib/vsprintf.c +2 -2

index 4e8f3e9acb99..e2a1d89f1a5c 100644
@@ -1781,8 +1781,8 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 		*p++ = isascii(c) && isprint(c) ? c : '.';
 	}
 
-	strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian");
-	p += strlen(p);
+	*p++ = ' ';
+	p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
 
 	*p++ = ' ';
 	*p++ = '(';

Signature · 1 line

2.34.1

kernel test robot

build bot

2022-01-24 19:23 UTC #2 lore raw

Re: [PATCH v2 2/2] vsprintf: Move space out of string literals in fourcc_string()

I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.17-rc1 next-20220124] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: arc-randconfig-r026-20220124 (https://download.01.org/0day-ci/archive/20220125/202201250147.uYccwqwy-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f1895edcabdc5dae0528ffff9c3cb665a937223d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420 git checkout f1895edcabdc5dae0528ffff9c3cb665a937223d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

lib/vsprintf.c: In function 'va_format':
lib/vsprintf.c:1695:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]

    1695 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~
   lib/vsprintf.c: In function 'fourcc_string':
>> lib/vsprintf.c:1794:13: error: implicit declaration of function 'stpcpy'; did you mean 'strcpy'? [-Werror=implicit-function-declaration]
    1794 |         p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
         |             ^~~~~~
         |             strcpy
>> lib/vsprintf.c:1794:11: warning: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
    1794 |         p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
         |           ^

cc1: some warnings being treated as errors

vim +1794 lib/vsprintf.c

1685
1686 static char *va_format(char *buf, char *end, struct va_format *va_fmt,
1687 struct printf_spec spec, const char *fmt)
1688 {
1689 va_list va;
1690
1691 if (check_pointer(&buf, end, va_fmt, spec))
1692 return buf;
1693
1694 va_copy(va, *va_fmt->va);

> 1695		buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
  1696		va_end(va);
  1697	
  1698		return buf;
  1699	}
  1700	
  1701	static noinline_for_stack
  1702	char *uuid_string(char *buf, char *end, const u8 *addr,
  1703			  struct printf_spec spec, const char *fmt)
  1704	{
  1705		char uuid[UUID_STRING_LEN + 1];
  1706		char *p = uuid;
  1707		int i;
  1708		const u8 *index = uuid_index;
  1709		bool uc = false;
  1710	
  1711		if (check_pointer(&buf, end, addr, spec))
  1712			return buf;
  1713	
  1714		switch (*(++fmt)) {
  1715		case 'L':
  1716			uc = true;
  1717			fallthrough;
  1718		case 'l':
  1719			index = guid_index;
  1720			break;
  1721		case 'B':
  1722			uc = true;
  1723			break;
  1724		}
  1725	
  1726		for (i = 0; i < 16; i++) {
  1727			if (uc)
  1728				p = hex_byte_pack_upper(p, addr[index[i]]);
  1729			else
  1730				p = hex_byte_pack(p, addr[index[i]]);
  1731			switch (i) {
  1732			case 3:
  1733			case 5:
  1734			case 7:
  1735			case 9:
  1736				*p++ = '-';
  1737				break;
  1738			}
  1739		}
  1740	
  1741		*p = 0;
  1742	
  1743		return string_nocheck(buf, end, uuid, spec);
  1744	}
  1745	
  1746	static noinline_for_stack
  1747	char *netdev_bits(char *buf, char *end, const void *addr,
  1748			  struct printf_spec spec,  const char *fmt)
  1749	{
  1750		unsigned long long num;
  1751		int size;
  1752	
  1753		if (check_pointer(&buf, end, addr, spec))
  1754			return buf;
  1755	
  1756		switch (fmt[1]) {
  1757		case 'F':
  1758			num = *(const netdev_features_t *)addr;
  1759			size = sizeof(netdev_features_t);
  1760			break;
  1761		default:
  1762			return error_string(buf, end, "(%pN?)", spec);
  1763		}
  1764	
  1765		return special_hex_number(buf, end, num, size);
  1766	}
  1767	
  1768	static noinline_for_stack
  1769	char *fourcc_string(char *buf, char *end, const u32 *fourcc,
  1770			    struct printf_spec spec, const char *fmt)
  1771	{
  1772		char output[sizeof("0123 little-endian (0x01234567)")];
  1773		char *p = output;
  1774		unsigned int i;
  1775		u32 orig, val;
  1776	
  1777		if (fmt[1] != 'c' || fmt[2] != 'c')
  1778			return error_string(buf, end, "(%p4?)", spec);
  1779	
  1780		if (check_pointer(&buf, end, fourcc, spec))
  1781			return buf;
  1782	
  1783		orig = get_unaligned(fourcc);
  1784		val = orig & ~BIT(31);
  1785	
  1786		for (i = 0; i < sizeof(u32); i++) {
  1787			unsigned char c = val >> (i * 8);
  1788	
  1789			/* Print non-control ASCII characters as-is, dot otherwise */
  1790			*p++ = isascii(c) && isprint(c) ? c : '.';
  1791		}
  1792	
  1793		*p++ = ' ';
> 1794		p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");

1795
1796 *p++ = ' ';
1797 *p++ = '(';
1798 p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
1799 *p++ = ')';
1800 *p = '\0';
1801
1802 return string(buf, end, output, spec);
1803 }
1804

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

kernel test robot

build bot

2022-01-24 19:23 UTC #3 lore raw

Re: [PATCH v2 2/2] vsprintf: Move space out of string literals in fourcc_string()

In reply to Andy Shevchenko

I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.17-rc1 next-20220124] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: riscv-randconfig-r042-20220124 (https://download.01.org/0day-ci/archive/20220125/202201250142.DtHQRlOS-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/f1895edcabdc5dae0528ffff9c3cb665a937223d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420 git checkout f1895edcabdc5dae0528ffff9c3cb665a937223d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> lib/vsprintf.c:1794:6: error: implicitly declaring library function 'stpcpy' with type 'char *(char *, const char *)' [-Werror,-Wimplicit-function-declaration]
           p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
               ^

lib/vsprintf.c:1794:6: note: include the header <string.h> or explicitly provide a declaration for 'stpcpy' 1 error generated.

vim +1794 lib/vsprintf.c

1767
1768 static noinline_for_stack
1769 char *fourcc_string(char *buf, char *end, const u32 *fourcc,
1770 struct printf_spec spec, const char *fmt)
1771 {
1772 char output[sizeof("0123 little-endian (0x01234567)")];
1773 char *p = output;
1774 unsigned int i;
1775 u32 orig, val;
1776
1777 if (fmt[1] != 'c' || fmt[2] != 'c')
1778 return error_string(buf, end, "(%p4?)", spec);
1779
1780 if (check_pointer(&buf, end, fourcc, spec))
1781 return buf;
1782
1783 orig = get_unaligned(fourcc);
1784 val = orig & ~BIT(31);
1785
1786 for (i = 0; i < sizeof(u32); i++) {
1787 unsigned char c = val >> (i * 8);
1788
1789 /* Print non-control ASCII characters as-is, dot otherwise */
1790 *p++ = isascii(c) && isprint(c) ? c : '.';
1791 }
1792
1793 *p++ = ' ';

> 1794		p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");

1795
1796 *p++ = ' ';
1797 *p++ = '(';
1798 p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
1799 *p++ = ')';
1800 *p = '\0';
1801
1802 return string(buf, end, output, spec);
1803 }
1804

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

kernel test robot

build bot

2022-01-24 19:23 UTC #4 lore raw

Re: [PATCH v2 2/2] vsprintf: Move space out of string literals in fourcc_string()

In reply to Andy Shevchenko

I love your patch! Perhaps something to improve:

Hi Andy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.17-rc1 next-20220124] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39
config: arm-randconfig-r022-20220124 (https://download.01.org/0day-ci/archive/20220125/202201250258.qVWkKB2I-lkp(a)intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f1895edcabdc5dae0528ffff9c3cb665a937223d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Shevchenko/vsprintf-Fix-potential-unaligned-access/20220124-224420 git checkout f1895edcabdc5dae0528ffff9c3cb665a937223d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

lib/vsprintf.c: In function 'va_format':
lib/vsprintf.c:1695:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]

    1695 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~
   lib/vsprintf.c: In function 'fourcc_string':
   lib/vsprintf.c:1794:13: error: implicit declaration of function 'stpcpy' [-Werror=implicit-function-declaration]
    1794 |         p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
         |             ^~~~~~
>> lib/vsprintf.c:1794:13: warning: incompatible implicit declaration of built-in function 'stpcpy' [-Wbuiltin-declaration-mismatch]

cc1: some warnings being treated as errors

vim +/stpcpy +1794 lib/vsprintf.c

1685
1686 static char *va_format(char *buf, char *end, struct va_format *va_fmt,
1687 struct printf_spec spec, const char *fmt)
1688 {
1689 va_list va;
1690
1691 if (check_pointer(&buf, end, va_fmt, spec))
1692 return buf;
1693
1694 va_copy(va, *va_fmt->va);

> 1695		buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
  1696		va_end(va);
  1697	
  1698		return buf;
  1699	}
  1700	
  1701	static noinline_for_stack
  1702	char *uuid_string(char *buf, char *end, const u8 *addr,
  1703			  struct printf_spec spec, const char *fmt)
  1704	{
  1705		char uuid[UUID_STRING_LEN + 1];
  1706		char *p = uuid;
  1707		int i;
  1708		const u8 *index = uuid_index;
  1709		bool uc = false;
  1710	
  1711		if (check_pointer(&buf, end, addr, spec))
  1712			return buf;
  1713	
  1714		switch (*(++fmt)) {
  1715		case 'L':
  1716			uc = true;
  1717			fallthrough;
  1718		case 'l':
  1719			index = guid_index;
  1720			break;
  1721		case 'B':
  1722			uc = true;
  1723			break;
  1724		}
  1725	
  1726		for (i = 0; i < 16; i++) {
  1727			if (uc)
  1728				p = hex_byte_pack_upper(p, addr[index[i]]);
  1729			else
  1730				p = hex_byte_pack(p, addr[index[i]]);
  1731			switch (i) {
  1732			case 3:
  1733			case 5:
  1734			case 7:
  1735			case 9:
  1736				*p++ = '-';
  1737				break;
  1738			}
  1739		}
  1740	
  1741		*p = 0;
  1742	
  1743		return string_nocheck(buf, end, uuid, spec);
  1744	}
  1745	
  1746	static noinline_for_stack
  1747	char *netdev_bits(char *buf, char *end, const void *addr,
  1748			  struct printf_spec spec,  const char *fmt)
  1749	{
  1750		unsigned long long num;
  1751		int size;
  1752	
  1753		if (check_pointer(&buf, end, addr, spec))
  1754			return buf;
  1755	
  1756		switch (fmt[1]) {
  1757		case 'F':
  1758			num = *(const netdev_features_t *)addr;
  1759			size = sizeof(netdev_features_t);
  1760			break;
  1761		default:
  1762			return error_string(buf, end, "(%pN?)", spec);
  1763		}
  1764	
  1765		return special_hex_number(buf, end, num, size);
  1766	}
  1767	
  1768	static noinline_for_stack
  1769	char *fourcc_string(char *buf, char *end, const u32 *fourcc,
  1770			    struct printf_spec spec, const char *fmt)
  1771	{
  1772		char output[sizeof("0123 little-endian (0x01234567)")];
  1773		char *p = output;
  1774		unsigned int i;
  1775		u32 orig, val;
  1776	
  1777		if (fmt[1] != 'c' || fmt[2] != 'c')
  1778			return error_string(buf, end, "(%p4?)", spec);
  1779	
  1780		if (check_pointer(&buf, end, fourcc, spec))
  1781			return buf;
  1782	
  1783		orig = get_unaligned(fourcc);
  1784		val = orig & ~BIT(31);
  1785	
  1786		for (i = 0; i < sizeof(u32); i++) {
  1787			unsigned char c = val >> (i * 8);
  1788	
  1789			/* Print non-control ASCII characters as-is, dot otherwise */
  1790			*p++ = isascii(c) && isprint(c) ? c : '.';
  1791		}
  1792	
  1793		*p++ = ' ';
> 1794		p = stpcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");

1795
1796 *p++ = ' ';
1797 *p++ = '(';
1798 p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
1799 *p++ = ')';
1800 *p = '\0';
1801
1802 return string(buf, end, output, spec);
1803 }
1804

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org