netdev - BUG solos-pci /sys parameters can't be accessed

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>] [day] [month] [year] [list]

Date:   Mon, 03 Jun 2019 16:53:44 +0100
From:   Iain Paton <a8b707e26f6083d@...gu.darkvoyage.org.uk>
To:     3chas3@...il.com, linux-atm-general@...ts.sourceforge.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        clabbe.montjoie@...il.com
CC:     nathan@...verse.com.au, dwmw2@...radead.org
Subject: BUG solos-pci /sys parameters can't be accessed

commit e94d91a6eb155ff77110863d15ba51b3c6b5c548 
atm: solos-pci: Replace simple_strtol by kstrtoint
causes access to any solos parameters under /sys/class/atm/solos-pci[n]/parameters
to fail, for example:

root@...os:/sys/class/atm/solos-pci0/parameters# cat State
cat: State: Input/output error

loading the module with atmdebug=1 shows that communication with the 
card is working

[  296.599161] solos 0000:03:01.0: Transmitted: port 0
[  296.599168] solos 0000:03:01.0: size: 13 VPI: 0 VCI: 0
[  296.599176] 00: 4C 30 31 31 31 38 0A 53 
[  296.599181] 08: 74 61 74 65 0A 

[  296.616012] solos 0000:03:01.0: Received: port 0
[  296.616027] solos 0000:03:01.0: size: 17 VPI: 0 VCI: 0
[  296.616039] 00: 4C 30 31 31 31 38 0A 48 
[  296.616049] 08: 61 6E 64 53 68 61 6B 65 
[  296.616052] 10: 0A 

and we're receiving the expected response.

The reason is in the following section

@@ -428,7 +432,9 @@ static int process_command(struct solos_card *card, int port, struct sk_buff *sk
            skb->data[6] != '\n')
                return 0;
 
-       cmdpid = simple_strtol(&skb->data[1], NULL, 10);
+       err = kstrtoint(&skb->data[1], 10, &cmdpid);
+       if (err)
+               return err;
 
        spin_lock_irqsave(&card->param_queue_lock, flags);
        list_for_each_entry(prm, &card->param_queue, list) {

as kstrtoint want's the input string to be as follows:

"The string must be null-terminated, and may also include a single 
newline before its terminating null. The first character may also be 
a plus sign or a minus sign."

this usage of kstrtoint will always fail as what's being passed in is 
not a simple null terminated string, rather it's a multi-value string 
where each value is seperated with newlines.

Reverting the patch sorts it, but doesn't really seem like the right 
thing to do.

Iain