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 <[email protected]> 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 >
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 <[email protected]> +# +# 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
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
