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-next>] [day] [month] [year] [list]
Date:	Fri, 15 Jan 2016 18:41:27 +0100
From:	Peter Senna Tschudin <peter.senna@...labora.com>
To:	thomas@...ischhofer.net, gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	joe@...ches.com, sergei.shtylyov@...entembedded.com
Cc:	Peter Senna Tschudin <peter.senna@...labora.com>
Subject: [PATCH V2 0/7] usb-misc: sisusbvga: cleanup and bug fix

The file drivers/usb/misc/sisusbvga/sisusb.c had many (192) coding style issues
reported by checkpatch. This file also had a problematic error path in the
probe function that could result in dereferencing a null pointer.

This patch series fix coding style issues and a problematic error path which
could result in a null pointer dereference.

Patch 1 and 2 change whitespace only, patch 3 to 6 fix various coding style
issues, and patch 7 fix a null pointer dereference bug.

Joe Perches suggested me to include objtdiff output for patches that are not
supposed to make semantic changes, but it is not working well for me with gcc
(GCC) 5.3.1 20151207 (Red Hat 5.3.1-2). Even compiling the same source code
produces different output from objdump. The objdump command I'm using is from
./scripts/objdiff. See an example:

# A patch that should not make any semantic change
$ cat /tmp/patch
 diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c
 index 8efbaba..a48b086d 100644
 --- a/drivers/usb/misc/sisusbvga/sisusb.c
 +++ b/drivers/usb/misc/sisusbvga/sisusb.c
 @@ -1353,7 +1353,7 @@ sisusb_testreadwrite(struct sisusb_usb_data *sisusb)
      static char srcbuffer[] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77 };
      char destbuffer[10];
      size_t dummy;
 -    int i,j;
 +    int i, j;
  
      sisusb_copy_memory(sisusb, srcbuffer, sisusb->vrambase, 7, &dummy);

# Lets compile and collect a dump based on linux-next/master
$ git checkout linux-next/master
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
d6ffbd44f3f1cf81fd55ce84441ab889  drivers/usb/misc/sisusbvga/sisusb.c

$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
  sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump

# Now let's apply the patch and collect other dump
$ git apply /tmp/patch
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
0b7d579c8ae2159f677c6a5c6efc4956  drivers/usb/misc/sisusbvga/sisusb.c

$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
  sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/dump

# I was expecting the diff to be empty
$ diff /tmp/base_dump /tmp/dump
 9135,9136c9135
 < :	8e 4d 31             	mov    0x31(%rbp),%cs
 < :	46 00 00             	rex.RX add %r8b,(%rax)
 ---
 > :	25 c4 31 46 00       	and    $0x4631c4,%eax
 9139c9138
 < :	4b 00 00             	rex.WXB add %al,(%r8)
 ---
 > :	00 4b 00             	add    %cl,0x0(%rbx)

# But here is the interesting part. Even compiling the exact same source code
# produces different results
$ git checkout -- .
$ md5sum drivers/usb/misc/sisusbvga/sisusb.c
d6ffbd44f3f1cf81fd55ce84441ab889  drivers/usb/misc/sisusbvga/sisusb.c

$ make -j4 drivers/usb/misc/sisusbvga/sisusb.o
$ objdump -D drivers/usb/misc/sisusbvga/sisusb.o| \
  sed "s/^[[:space:]]\+[0-9a-f]\+//" > /tmp/base_dump_again

$ diff /tmp/base_dump /tmp/base_dump_again 
 9135,9136c9135,9136
 < :	8e 4d 31             	mov    0x31(%rbp),%cs
 < :	46 00 00             	rex.RX add %r8b,(%rax)
 ---
 > :	de 10                	ficom  (%rax)
 > :	33 46 00             	xor    0x0(%rsi),%eax
 9139c9139
 < :	4b 00 00             	rex.WXB add %al,(%r8)
 ---
 > :	00 4b 00             	add    %cl,0x0(%rbx)

Peter Senna Tschudin (7):
  usb-misc: sisusbvga: Fix coding style: horizontal whitespace changes
  usb-misc: sisusbvga: Fix coding style: vertical whitespace changes
  usb-misc: sisusbvga: Fix coding style: braces, parenthesis, comment
  usb-misc: sisusbvga: Fix coding style: remove assignment from if tests
  usb-misc: sisusbvga: Remove null test before calls to kfree()
  usb-misc: sisusbvga: Remove memory allocation logs
  usb-misc: sisusbvga: fix error path

 drivers/usb/misc/sisusbvga/sisusb.c | 1543 +++++++++++++++++------------------
 1 file changed, 752 insertions(+), 791 deletions(-)

-- 
2.5.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ