[patch] various bugs and strncpy abuse followup
Tito
farmatito at tiscali.it
Tue Jun 6 23:43:46 PDT 2006
On Wednesday 7 June 2006 00:34, Erik Hovland wrote:
> After the very helpful feedback and the continual searching for bugs, I
> have expanded my patch associated to various bugs all over busybox.
>
> This patchset has gotten bigger. So I have taken to splitting them up by
> file and annotating them in patch. I hope that is useful. Let me know if
> there is some way I can make this easier to go over or help with
> acceptance.
>
> Feel free to address each patch seperately in responding. I would not be
> surprised at all if I accidentally introduced regressions. I have not
> yet tried running the unit testing. But I have attempted to understand
> why each bug is a bug and addressed it according to its severity.
>
> I appreciate any attention you might give this work.
>
Hi,
some comments about the devfsd patch:
1)
--- miscutils/devfsd.c (revision 15298)
+++ miscutils/devfsd.c (working copy)
@@ -964,8 +964,9 @@
regexpr, numexpr);
if ( !make_dir_tree (destination) || lstat (source, &source_stat) != 0)
- return;
- lstat (destination, &dest_stat);
+ return;
+ if (lstat (destination, &dest_stat) != 0)
+ return;;
new_mode = source_stat.st_mode & ~S_ISVTX;
if (info->type == DEVFSD_NOTIFY_CREATE)
new_mode |= S_ISVTX;
This is unneeded. The original code does:
if (lstat (destination, &dest_stat) != 0) dest_stat.st_mode = 0;
but we have already initialised dest_stat.st_mode to 0 so we can spare the if statement.
Please drop this part.
2)
@@ -1090,7 +1091,8 @@
dest_stat.st_mode = 0;
snprintf (dpath, sizeof dpath, "%s%s", mount_point, spath + rootlen);
- lstat (dpath, &dest_stat);
+ if (lstat (dpath, &dest_stat) < 0)
+ bb_error_msg("Unable to stat %s", dpath);
if ( S_ISLNK (source_stat.st_mode) || (source_stat.st_mode & S_ISVTX) )
copy_inode (dpath, &dest_stat, (source_stat.st_mode & ~S_ISVTX) , spath, &source_stat);
Originally was:
}
snprintf (dpath, sizeof dpath, "%s%s", mount_point, spath + rootlen);
if (lstat (dpath, &dest_stat) != 0) dest_stat.st_mode = 0;
Same as above. Please drop this part.
3)
@@ -1311,9 +1313,9 @@
/* compare_string_array returns i>=0 */
i=compare_string_array(field_names, variable);
Good catch, at the time I wrote this compare_string_array didn't return values < 0.
BTW, if I remember correctly here it is unlikely to happen, but who knows.....
Maybe we should fix also the comment or remove it totally.
- if ( i > 6 && (i > 1 && gv_info == NULL))
- return (NULL);
- if( i >= 0 || i <= 3)
+ if (i > 6 || (i > 1 && gv_info == NULL) || i < 0)
+ return (NULL);
Good catch too!!!
+ if( i <= 3 && i >= 0 )
{
debug_msg_logger(LOG_INFO, "%s: i=%d %s", __FUNCTION__, i ,field_names[i+7]);
return(field_names[i+7]);
Thanks for auditing this old code.
Ciao,
Tito
More information about the busybox
mailing list