Re: [U-Boot] Prevent null pointer dereference originating in cmd_pxe.c

2013-10-14 Thread Tom Rini 
On Mon, Oct 07, 2013 at 09:51:48AM -0400, Steven Falco wrote:

 Pass a valid cmdtp into do_tftpb(), do_ext2load(), and do_get_fat(), to avoid
 possible crashes due to null pointer dereferencing.
 
 Signed-off-by: Steven A. Falco stevenfa...@gmail.com
 
 ---
  This doesn't apply cleanly, nor with --ignore-whitespace for me.  Can
  you please re-check and re-send the patch?  Thanks.
 
 Sorry - I've been having trouble getting Thunderbird to leave my text
 alone.  There was some insane flowed text setting that I just discovered
 and disabled.
 
 I think I've got it right now.  I'll download this email from the list
 after I post it, and do a diff to be sure.
 
 Commit d7884e047d08447dfd1374e9fa2fdf7ab36e56f5 does not go far enough.  There
 is still at least one call chain that can result in a crash.
 
 The do_tftpb(), do_ext2load(), and do_get_fat() functions expect a valid 
 cmdtp.
 Passing in NULL is particularly bad in the do_tftpb() case, because eventually
 boot_get_kernel() will be called with a NULL cmdtp:
 
 do_tftpb() - netboot_common() - bootm_maybe_autostart() - do_bootm() -
 do_bootm_states() - bootm_find_os() - boot_get_kernel()
 
 Around line 991 in cmd_bootm.c, boot_get_kernel() will dereference the null
 pointer, and the board will crash.

With a reworded commit message to include more details, applied to
u-boot/master, thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Prevent null pointer dereference originating in cmd_pxe.c

2013-10-07 Thread Tom Rini 
On Wed, Sep 25, 2013 at 09:35:30PM -0400, Steven Falco wrote:

 Pass a valid cmdtp into do_tftpb(), do_ext2load(), and do_get_fat(), to avoid
 possible crashes due to null pointer dereferencing.
 
 Signed-off-by: Steven A. Falco stevenfalco at gmail.com

This doesn't apply cleanly, nor with --ignore-whitespace for me.  Can
you please re-check and re-send the patch?  Thanks.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Prevent null pointer dereference originating in cmd_pxe.c

2013-10-07 Thread Steven Falco 
Pass a valid cmdtp into do_tftpb(), do_ext2load(), and do_get_fat(), to avoid
possible crashes due to null pointer dereferencing.

Signed-off-by: Steven A. Falco stevenfa...@gmail.com

---

 This doesn't apply cleanly, nor with --ignore-whitespace for me.  Can
 you please re-check and re-send the patch?  Thanks.

Sorry - I've been having trouble getting Thunderbird to leave my text
alone.  There was some insane flowed text setting that I just discovered
and disabled.

I think I've got it right now.  I'll download this email from the list
after I post it, and do a diff to be sure.

Commit d7884e047d08447dfd1374e9fa2fdf7ab36e56f5 does not go far enough.  There
is still at least one call chain that can result in a crash.

The do_tftpb(), do_ext2load(), and do_get_fat() functions expect a valid cmdtp.
Passing in NULL is particularly bad in the do_tftpb() case, because eventually
boot_get_kernel() will be called with a NULL cmdtp:

do_tftpb() - netboot_common() - bootm_maybe_autostart() - do_bootm() -
do_bootm_states() - bootm_find_os() - boot_get_kernel()

Around line 991 in cmd_bootm.c, boot_get_kernel() will dereference the null
pointer, and the board will crash.

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index c5f4a22..79d3a06 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -114,16 +114,16 @@ static int get_bootfile_path(const char *file_path, char 
*bootfile_path,
return 1;
 }
 
-static int (*do_getfile)(const char *file_path, char *file_addr);
+static int (*do_getfile)(cmd_tbl_t *cmdtp, const char *file_path, char 
*file_addr);
 
-static int do_get_tftp(const char *file_path, char *file_addr)
+static int do_get_tftp(cmd_tbl_t *cmdtp, const char *file_path, char 
*file_addr)
 {
char *tftp_argv[] = {tftp, NULL, NULL, NULL};
 
tftp_argv[1] = file_addr;
tftp_argv[2] = (void *)file_path;
 
-   if (do_tftpb(NULL, 0, 3, tftp_argv))
+   if (do_tftpb(cmdtp, 0, 3, tftp_argv))
return -ENOENT;
 
return 1;
@@ -131,27 +131,27 @@ static int do_get_tftp(const char *file_path, char 
*file_addr)
 
 static char *fs_argv[5];
 
-static int do_get_ext2(const char *file_path, char *file_addr)
+static int do_get_ext2(cmd_tbl_t *cmdtp, const char *file_path, char 
*file_addr)
 {
 #ifdef CONFIG_CMD_EXT2
fs_argv[0] = ext2load;
fs_argv[3] = file_addr;
fs_argv[4] = (void *)file_path;
 
-   if (!do_ext2load(NULL, 0, 5, fs_argv))
+   if (!do_ext2load(cmdtp, 0, 5, fs_argv))
return 1;
 #endif
return -ENOENT;
 }
 
-static int do_get_fat(const char *file_path, char *file_addr)
+static int do_get_fat(cmd_tbl_t *cmdtp, const char *file_path, char *file_addr)
 {
 #ifdef CONFIG_CMD_FAT
fs_argv[0] = fatload;
fs_argv[3] = file_addr;
fs_argv[4] = (void *)file_path;
 
-   if (!do_fat_fsload(NULL, 0, 5, fs_argv))
+   if (!do_fat_fsload(cmdtp, 0, 5, fs_argv))
return 1;
 #endif
return -ENOENT;
@@ -165,7 +165,7 @@ static int do_get_fat(const char *file_path, char 
*file_addr)
  *
  * Returns 1 for success, or  0 on error.
  */
-static int get_relfile(const char *file_path, void *file_addr)
+static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void 
*file_addr)
 {
size_t path_len;
char relfile[MAX_TFTP_PATH_LEN+1];
@@ -194,7 +194,7 @@ static int get_relfile(const char *file_path, void 
*file_addr)
 
sprintf(addr_buf, %p, file_addr);
 
-   return do_getfile(relfile, addr_buf);
+   return do_getfile(cmdtp, relfile, addr_buf);
 }
 
 /*
@@ -204,13 +204,13 @@ static int get_relfile(const char *file_path, void 
*file_addr)
  *
  * Returns 1 on success, or  0 for error.
  */
-static int get_pxe_file(const char *file_path, void *file_addr)
+static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void 
*file_addr)
 {
unsigned long config_file_size;
char *tftp_filesize;
int err;
 
-   err = get_relfile(file_path, file_addr);
+   err = get_relfile(cmdtp, file_path, file_addr);
 
if (err  0)
return err;
@@ -241,7 +241,7 @@ static int get_pxe_file(const char *file_path, void 
*file_addr)
  *
  * Returns 1 on success or  0 on error.
  */
-static int get_pxelinux_path(const char *file, void *pxefile_addr_r)
+static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void 
*pxefile_addr_r)
 {
size_t base_len = strlen(PXELINUX_DIR);
char path[MAX_TFTP_PATH_LEN+1];
@@ -254,7 +254,7 @@ static int get_pxelinux_path(const char *file, void 
*pxefile_addr_r)
 
sprintf(path, PXELINUX_DIR %s, file);
 
-   return get_pxe_file(path, pxefile_addr_r);
+   return get_pxe_file(cmdtp, path, pxefile_addr_r);
 }
 
 /*
@@ -262,7 +262,7 @@ static int get_pxelinux_path(const char *file, void 
*pxefile_addr_r)
  *
  * Returns 1 on success or  0 on error.
  */
-static int pxe_uuid_path(void *pxefile_addr_r)
+static int pxe_uuid_path(cmd_tbl_t *cmdtp, void

Re: [U-Boot] Prevent null pointer dereference originating in cmd_pxe.c

2013-10-07 Thread Jagan Teki 
It's good to write commit message head for easy to track the fix from where.

cmd_pxe: Prevent null pointer deference

For more info, please look at here.
http://www.denx.de/wiki/U-Boot/Patches


On Mon, Oct 7, 2013 at 7:21 PM, Steven Falco stevenfa...@gmail.com wrote:
 Pass a valid cmdtp into do_tftpb(), do_ext2load(), and do_get_fat(), to avoid
 possible crashes due to null pointer dereferencing.

 Signed-off-by: Steven A. Falco stevenfa...@gmail.com

 ---

 This doesn't apply cleanly, nor with --ignore-whitespace for me.  Can
 you please re-check and re-send the patch?  Thanks.

 Sorry - I've been having trouble getting Thunderbird to leave my text
 alone.  There was some insane flowed text setting that I just discovered
 and disabled.

 I think I've got it right now.  I'll download this email from the list
 after I post it, and do a diff to be sure.

 Commit d7884e047d08447dfd1374e9fa2fdf7ab36e56f5 does not go far enough.  There
 is still at least one call chain that can result in a crash.

 The do_tftpb(), do_ext2load(), and do_get_fat() functions expect a valid 
 cmdtp.
 Passing in NULL is particularly bad in the do_tftpb() case, because eventually
 boot_get_kernel() will be called with a NULL cmdtp:

 do_tftpb() - netboot_common() - bootm_maybe_autostart() - do_bootm() -
 do_bootm_states() - bootm_find_os() - boot_get_kernel()

 Around line 991 in cmd_bootm.c, boot_get_kernel() will dereference the null
 pointer, and the board will crash.

 diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
 index c5f4a22..79d3a06 100644
 --- a/common/cmd_pxe.c
 +++ b/common/cmd_pxe.c
 @@ -114,16 +114,16 @@ static int get_bootfile_path(const char *file_path, 
 char *bootfile_path,
 return 1;
  }

 -static int (*do_getfile)(const char *file_path, char *file_addr);
 +static int (*do_getfile)(cmd_tbl_t *cmdtp, const char *file_path, char 
 *file_addr);

 -static int do_get_tftp(const char *file_path, char *file_addr)
 +static int do_get_tftp(cmd_tbl_t *cmdtp, const char *file_path, char 
 *file_addr)
  {
 char *tftp_argv[] = {tftp, NULL, NULL, NULL};

 tftp_argv[1] = file_addr;
 tftp_argv[2] = (void *)file_path;

 -   if (do_tftpb(NULL, 0, 3, tftp_argv))
 +   if (do_tftpb(cmdtp, 0, 3, tftp_argv))
 return -ENOENT;

 return 1;
 @@ -131,27 +131,27 @@ static int do_get_tftp(const char *file_path, char 
 *file_addr)

  static char *fs_argv[5];

 -static int do_get_ext2(const char *file_path, char *file_addr)
 +static int do_get_ext2(cmd_tbl_t *cmdtp, const char *file_path, char 
 *file_addr)
  {
  #ifdef CONFIG_CMD_EXT2
 fs_argv[0] = ext2load;
 fs_argv[3] = file_addr;
 fs_argv[4] = (void *)file_path;

 -   if (!do_ext2load(NULL, 0, 5, fs_argv))
 +   if (!do_ext2load(cmdtp, 0, 5, fs_argv))
 return 1;
  #endif
 return -ENOENT;
  }

 -static int do_get_fat(const char *file_path, char *file_addr)
 +static int do_get_fat(cmd_tbl_t *cmdtp, const char *file_path, char 
 *file_addr)
  {
  #ifdef CONFIG_CMD_FAT
 fs_argv[0] = fatload;
 fs_argv[3] = file_addr;
 fs_argv[4] = (void *)file_path;

 -   if (!do_fat_fsload(NULL, 0, 5, fs_argv))
 +   if (!do_fat_fsload(cmdtp, 0, 5, fs_argv))
 return 1;
  #endif
 return -ENOENT;
 @@ -165,7 +165,7 @@ static int do_get_fat(const char *file_path, char 
 *file_addr)
   *
   * Returns 1 for success, or  0 on error.
   */
 -static int get_relfile(const char *file_path, void *file_addr)
 +static int get_relfile(cmd_tbl_t *cmdtp, const char *file_path, void 
 *file_addr)
  {
 size_t path_len;
 char relfile[MAX_TFTP_PATH_LEN+1];
 @@ -194,7 +194,7 @@ static int get_relfile(const char *file_path, void 
 *file_addr)

 sprintf(addr_buf, %p, file_addr);

 -   return do_getfile(relfile, addr_buf);
 +   return do_getfile(cmdtp, relfile, addr_buf);
  }

  /*
 @@ -204,13 +204,13 @@ static int get_relfile(const char *file_path, void 
 *file_addr)
   *
   * Returns 1 on success, or  0 for error.
   */
 -static int get_pxe_file(const char *file_path, void *file_addr)
 +static int get_pxe_file(cmd_tbl_t *cmdtp, const char *file_path, void 
 *file_addr)
  {
 unsigned long config_file_size;
 char *tftp_filesize;
 int err;

 -   err = get_relfile(file_path, file_addr);
 +   err = get_relfile(cmdtp, file_path, file_addr);

 if (err  0)
 return err;
 @@ -241,7 +241,7 @@ static int get_pxe_file(const char *file_path, void 
 *file_addr)
   *
   * Returns 1 on success or  0 on error.
   */
 -static int get_pxelinux_path(const char *file, void *pxefile_addr_r)
 +static int get_pxelinux_path(cmd_tbl_t *cmdtp, const char *file, void 
 *pxefile_addr_r)
  {
 size_t base_len = strlen(PXELINUX_DIR);
 char path[MAX_TFTP_PATH_LEN+1];
 @@ -254,7 +254,7 @@ static int get_pxelinux_path(const char *file, void 
 *pxefile_addr_r)

 sprintf(path,

Re: [U-Boot] Prevent null pointer dereference originating in cmd_pxe.c

2013-10-07 Thread Fabio Estevam 
On Mon, Oct 7, 2013 at 10:51 AM, Steven Falco stevenfa...@gmail.com wrote:

 Sorry - I've been having trouble getting Thunderbird to leave my text
 alone.  There was some insane flowed text setting that I just discovered
 and disabled.

Better send patches via 'git send-email' as this can avoid these kind
of email client issues.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot