[patch] various bugs and strncpy abuse followup

Rob Landley rob at landley.net
Sun Jun 25 15:39:44 PDT 2006


dhcpc.c: Dropped.  The value of "lease" default is set to 1 hour on line 503, 
when a lease packet is read.   The reason the variable isn't given a value 
when it's declared is that whatever value it has will be overwritten later.  
(Flow control's a bit funky, the _first_ switch statement handles timeouts 
and the else case is what actually reads packets (line 418).  Flow control is 
determined by the global variable "state" which starts at INIT_SELECTING and 
gets reset by various functions in response to incoming packets...)

ext_attr.c: Unfortunately if you do that line 55 breaks when the #ifdef is 
disabled, since that also uses it.  (Also, the compiler can do constant 
propogation and optimize out if (0) {} code blocks.)  But I did an unrelated 
cleanup while I was in the area...

fdisk.c: Fixed and applied.  You'd have to copy 9 to use memcpy there (your 
patch leaves it without a NUL terminator), but why not just use strcpy there 
and save an argument on the stack?  Which is what I did.  This is ugly code 
I'd love to get time to rewrite entirely someday...

flushb.c: Dropped.  This is an ioctl that only applies to floppy disks (other 
block devices use BLKFLSBUF and you'd _think_ 2.6 would have fixed that to 
work on floppies if anybody actually cared about them anymore), and in that 
context see the comment on line 47:
/*
 * This function will sync a device/file, and optionally attempt to
 * flush the buffer cache.  The latter is basically only useful for
 * system benchmarks and for torturing systems in burn-in tests.  :)
 */

getty.c: Wow this code is a mess, but you ask a good question: what does it 
mean if TIOCSPGRP fails?  Can that happen?  Let's see, in 2.6.17, 
drivers/char/tty_io.c, the function tiocspgrp() can 
return -ENOTTY, -EFAULT, -EINVAL, or -EPERM.  So basically if you feed it a 
bad pointer, aren't root, try to call it on something that isn't a tty, or 
haven't got a process group to associate it with.  Of these "aren't root" 
and "isn't a tty" are probably worth reporting, yeah.

On the other hand, report them _where_ (To the tty?  The console?  The system 
log?), and is exiting the correct response (which error() will do, notice 
it's declared ATTRIBUTE_NORETURN).

As for the strncpy->memcpy change, where did you get the idea ut_id wasn't a 
string?  Try "man 3 setutent" and look at the example code, and tracing back 
where we get "line" in our code, it's options.tty which comes from 
parse_args() which is getting it from the command line.

I did switch a few things to safe_strncpy() here on the grounds it couldn't 
hurt, so I suppose you could call bits of this one applied. :)

Rob
-- 
Never bet against the cheap plastic solution.


More information about the busybox mailing list