[Toybox] Scan-build error

[Cynt Rynt]

Hi,
Thanks for getting back to me so quickly.  I was trying to figure out how
to write coverage tests for toybox, and ran into issues using the
pointtopoint option for ifconfig.  Before I finished writing the test
suite, I thought it best to find out if I was calling ifconfig wrong or if
I found a bug.  Attached is the .patch that generated the issue.

Thanks,
Cindy


On Tue, Nov 11, 2014 at 1:07 PM, Rob Landley <rob at landley.net> wrote:

> On 11/11/14 13:05, Cynt Rynt wrote:
> > Hi,
> > Ran scan-build on toybox and found some errors.  This fix removed an
> > error in password.c, but I'm not sure if it's the right fix.
> > Thanks,
> > Cindy
>
> Thanks, let's take a look...
>
> I suspect the correct fix is just not to save the return code. It's a
> speculative unlink: the file may not be there to be unlinked (most
> likely it won't be, it's just a backup), and if it is still there the
> link() should fail and we process the error from that.
>
> Now it's possible that you could have a symlink you can't unlink, and
> thus the link might move the file to the wrong location (although it
> won't cross filesystems and the result would still have the original
> permissions), but if someone can create a specific file in /etc that
> _root_ can't remove, the system is already compromised.
>
> This function has a larger problem that it assumes colon separated
> /etc/passwd format and android doesn't use that. (Android decided that
> the windows system registry was a good idea, and made a big global
> database in a binary format for critical system information to live in,
> because reasons.)
>
> That's why I haven't done a cleanup pass on this function yet: I don't
> know what to do about the design issue. I need to set up an AOSP build
> environment, and my obvious machine to do that on is stuck on Ubuntu
> 13.04 because the build servers went down and it won't upgrade anymore,
> so I need to back it up and reinstall it...
>
> Thanks for the heads up, I checked in the quick one line fix. I'll have
> to take a proper look at this function after the 0.5.1 release on saturday.
>
> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20141112/ffeb21b3/attachment-0003.htm>
-------------- next part --------------
diff -r 2997847aa299 tests/ifconfig.test
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/ifconfig.test	Wed Nov 12 18:33:39 2014 -0800
@@ -0,0 +1,160 @@
+#!/bin/bash
+# Copyright 2014 Cynthia Rempel <cynthia at rtems.org>
+#
+# Brief: Some cursery coverage tests of ifconfig...
+# Note: requires permissions to run modprobe and all ifconfig options
+# Commands used: grep, grep -i, modeprobe, modprobe -r, wc -l
+# Resources modified: dummy0
+#
+# Possible improvements:
+# 1. Verify the dummy interface actually has the modified characteristics
+#    instead of relying on ifconfig output
+# 2. Introduce more error cases, to verify ifconfig fails gracefully
+# 3. Do more complex calls to ifconfig, while mixing the order of the
+#    arguments
+# 4. Cover more ifconfig options:
+#    hw ether|infiniband ADDRESS - set LAN hardware address (AA:BB:CC...)
+#    txqueuelen LEN - number of buffered packets before output blocks
+#    Obsolete fields included for historical purposes:
+#    irq|io_addr|mem_start ADDR - micromanage obsolete hardware
+#    outfill|keepalive INTEGER - SLIP analog dialup line quality monitoring
+#    metric INTEGER - added to Linux 0.9.10 with comment "never used", still true
+
+[ -f testing.sh ] && . testing.sh
+
+#testing "name" "command" "result" "infile" "stdin"
+
+# Add a dummy interface to test with
+modprobe dummy
+
+# Disable the dummy interface
+# ifconfig <interface> down
+testing "ifconfig dummy0 down and if config /-only" \
+"ifconfig dummy0 down && ifconfig dummy0 | grep dummy | wc -l" \
+"0\n" "" ""
+
+# Enable the dummy interface
+# ifconfig <interface> up
+testing "ifconfig dummy0 up" \
+"ifconfig dummy0 up && ifconfig dummy0 | grep dummy | wc -l" \
+"1\n" "" ""
+
+# Assign an ip address to the interface
+# ifconfig <interface> <address>
+testing "ifconfig dummy0 10.240.240.240" \
+"ifconfig dummy0 10.240.240.240 && ifconfig dummy0 | grep 10\.240\.240\.240 | wc -l" \
+"1\n" "" ""
+
+# Change the netmask to the interface
+# ifconfig <interface> netmask <address>
+testing "ifconfig dummy0 netmask 255.255.240.0" \
+"ifconfig dummy0 netmask 255.255.240.0 && ifconfig dummy0 | grep 255\.255\.240\.0 | wc -l" \
+"1\n" "" ""
+
+# Change the broadcast address to the interface
+# ifconfig <interface> broadcast <address>
+testing "ifconfig dummy0 broadcast 10.240.240.255" \
+"ifconfig dummy0 broadcast 10.240.240.255 && ifconfig dummy0 | grep 10\.240\.240\.255 | wc -l" \
+"1\n" "" ""
+
+# Change the point-to-point address to the interface
+# TODO: write a test-case for this
+
+# Revert to the default ip address
+testing "ifconfig dummy0 default" \
+"ifconfig dummy0 default && ifconfig dummy0 | grep 10\.240\.240\.240 | wc -l" \
+"0\n" "" ""
+
+# Change the Maximum transmission unit (MTU) of the interface
+# ifconfig <interface> mtu <#>
+testing "ifconfig dummy0 mtu 1269" \
+"ifconfig dummy0 mtu 1269 && ifconfig dummy0 | grep 1269 | wc -l" \
+"1\n" "" ""
+
+# Verify ifconfig add fails with such a small mtu
+testing "ifconfig dummy0 add ::2 -- too small mtu" \
+"ifconfig dummy0 add ::2 2>&1 | grep No\ buffer\ space\ available | wc -l" \
+"1\n" "" ""
+
+# Change the Maximum transmission unit (MTU) of the interface
+# ifconfig <interface> mtu <#>
+testing "ifconfig dummy0 mtu 2000" \
+"ifconfig dummy0 mtu 2000 && ifconfig dummy0 | grep 2000 | wc -l" \
+"1\n" "" ""
+
+# Verify ifconfig add succeeds with a larger mtu
+testing "ifconfig dummy0 add ::2" \
+"ifconfig dummy0 add ::2/126 && ifconfig dummy0 | grep \:\:2\/126 | wc -l" \
+"1\n" "" ""
+
+# Test ifconfig <interface> del <addr>
+testing "ifconfig dummy0 del ::2" \
+"ifconfig dummy0 del ::2/126 && ifconfig dummy0 | grep \:\:2 | wc -l" \
+"0\n" "" ""
+
+# Test two options at once
+testing "ifconfig dummy0 arp down" \
+"ifconfig dummy0 arp down && ifconfig dummy0 | grep -i NOARP | wc -l" \
+"0\n" "" ""
+
+# Test the pointtopoint option -- no argument
+testing "ifconfig dummy0 pointtopoint" \
+"ifconfig dummy0 pointtopoint && ifconfig dummy0 | grep -i NOARP | grep -i UP | wc -l" \
+"1\n" "" ""
+
+# Test the pointtopoint option -- one argument
+testing "ifconfig dummy0 pointtopoint 127.0.0.2" \
+"ifconfig dummy0 pointtopoint 127.0.0.2 && ifconfig dummy0 | grep -i inet | grep -i 127\.0\.0\.2 | wc -l" \
+"1\n" "" ""
+
+####### Flags you can set on an interface (or -remove by prefixing with -): ###############
+
+# Enable allmulti mode on the interface
+# ifconfig <interface> allmulti
+testing "ifconfig dummy0 allmulti" \
+"ifconfig dummy0 allmulti && ifconfig dummy0 | grep -i allmulti | wc -l" "1\n" \
+"" ""
+
+# Disable multicast mode the interface
+# ifconfig <interface> -multicast
+testing "ifconfig dummy0 -allmulti" \
+"ifconfig dummy0 -allmulti && ifconfig dummy0 | grep -i allmulti | wc -l" "0\n" \
+"" ""
+
+# Disable NOARP mode on the interface
+# ifconfig <interface> arp
+testing "ifconfig dummy0 arp" \
+"ifconfig dummy0 arp && ifconfig dummy0 | grep -i NOARP | wc -l" "0\n" \
+"" ""
+
+# Enable NOARP mode on the interface
+# ifconfig <interface> -arp
+testing "ifconfig dummy0 -arp" \
+"ifconfig dummy0 -arp && ifconfig dummy0 | grep -i NOARP | wc -l" "1\n" \
+"" ""
+
+# Enable multicast mode on the interface
+# ifconfig <interface> multicast
+testing "ifconfig dummy0 multicast" \
+"ifconfig dummy0 multicast && ifconfig dummy0 | grep -i multicast | wc -l" "1\n" \
+"" ""
+
+# Disable multicast mode the interface
+# ifconfig <interface> -multicast
+testing "ifconfig dummy0 -multicast" \
+"ifconfig dummy0 -multicast && ifconfig dummy0 | grep -i multicast | wc -l" "0\n" \
+"" ""
+
+# Enable promiscuous mode the interface
+# ifconfig <interface> promisc
+testing "ifconfig dummy0 promisc" \
+"ifconfig dummy0 promisc && ifconfig dummy0 | grep -i promisc | wc -l" "1\n" \
+"" ""
+
+# Disable promiscuous mode the interface
+# ifconfig <interface> -promisc
+testing "ifconfig dummy0 -promisc" \
+"ifconfig dummy0 -promisc && ifconfig dummy0 | grep -i promisc | wc -l" "0\n" \
+"" ""
+
+modprobe -r dummy