[PATCH] Fix 2 possible SEGVs in tftp client

[Jason Schoon]

On 6/13/06, Bernhard Fischer <rep.nop at aon.at> wrote:
> On Tue, Jun 13, 2006 at 02:20:54AM -0500, Jason Schoon wrote:
>
> >Attached is one more patch for this to fix up two minor issues.  A
> >duplicate const declaration, and my error in not checking if we were
> >using stdin or stdout before freeing the file handle.
>
> >--- busybox/networking/tftp.c.orig     2006-06-13 02:12:44.000000000 -0500
> >+++ busybox/networking/tftp.c  2006-06-13 02:13:55.000000000 -0500
> >@@ -95,7 +95,7 @@
> >       return blocksize;
> > }
> >
> >-static char *tftp_option_get(char *buf, int len, const char const *option)
> ->+static char *tftp_option_get(char *buf, int len, char const *option)
> +static char *tftp_option_get(char *buf, int len, const char * const option)
> > {
> >       int opt_val = 0;
> >       int opt_found = 0;
> >@@ -573,12 +573,12 @@
> >
> >       result = tftp(cmd, host, remotefile, fd, port, blocksize);
> >
> >-#ifdef CONFIG_FEATURE_CLEAN_UP
> >       if (!(fd == STDOUT_FILENO || fd == STDIN_FILENO)) {
> >+#ifdef CONFIG_FEATURE_CLEAN_UP
>
> Do i understand you right?
> if (ENABLE_FEATURE_CLEAN_UP) {
>         if (!(fd == STDOUT_FILENO || fd == STDIN_FILENO))
>                 close(fd);
>         if (cmd == tftp_cmd_get && result != EXIT_SUCCESS)
>                 unlink(localfile);
> }
> Why do you want to bypass feature_cleanup for rm'ing the failed file?
> Just curious..
>

I'll admit it was late when I made those changes, but I'm confused
now.  The feature_cleanup stuff shouldn't have changed, I just moved
the check for stdin/stdout outside of that ifdef because we want to
unlink the file in any error case, regardless of feature_cleanup being
enabled.  Moving the check outside of feature_cleanup saves checking
the file descriptors twice.

Checking....Yeah, it appears to do what I want.  Is there something
else I am missing?

When I grabbed the patch back off the mailing list (no local copy
here), it didn't seem to apply completely cleanly.  It possible either
my browser or something else horked it, but attached is one that I
just verified patches cleanly, just in case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox_tftp.patch
Type: text/x-patch
Size: 777 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060613/f569e4cc/attachment.bin