| Message ID | 20161111041302.17837-1-cyril.bur@au1.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
Cyril Bur <cyril.bur@au1.ibm.com> writes: > If the -F flag is used then pflash uses a regular file as the flash. > > On cleanup pflash fails to tell arch_flash_close() if it had passed a > filename to arch_flash_init() as such arch_flash_close() assumes that > it needs to close the actual flash structure and not simply a file > descriptor leading to a NULL dereference. > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > Note: This fix is correct but I'm not particularly fond of it. A > better solution is to refactor and stop relying on atexit() to perform > cleanup after the use of exit(). This cleanup would also benifit > debugging, this work and tests to avoid this situation in future is in > my work queue. Cool. I'll leave you to refactor it, but merge this now to master (as of 02a628a37982ebcc00f050d2a6a73aa6247718f2 ) so that it makse it into 5.4.0
On Fri, 2016-11-11 at 15:24 +1100, Stewart Smith wrote: > Cyril Bur <cyril.bur@au1.ibm.com> writes: > > If the -F flag is used then pflash uses a regular file as the > > flash. > > > > On cleanup pflash fails to tell arch_flash_close() if it had passed > > a > > filename to arch_flash_init() as such arch_flash_close() assumes > > that > > it needs to close the actual flash structure and not simply a file > > descriptor leading to a NULL dereference. > > > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > > --- > > Note: This fix is correct but I'm not particularly fond of it. A > > better solution is to refactor and stop relying on atexit() to > > perform > > cleanup after the use of exit(). This cleanup would also benifit > > debugging, this work and tests to avoid this situation in future is > > in > > my work queue. > > Cool. I'll leave you to refactor it, but merge this now to master (as > of > 02a628a37982ebcc00f050d2a6a73aa6247718f2 ) so that it makse it into > 5.4.0 Thanks. >
diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c index c93bbd4..fb45870 100644 --- a/external/pflash/pflash.c +++ b/external/pflash/pflash.c @@ -42,6 +42,7 @@ /* Full pflash version number (possibly includes gitid). */ extern const char version[]; +const char *flashfilename = NULL; static bool must_confirm = true; static bool dummy_run; static int need_relock; @@ -524,7 +525,7 @@ void exiting(void) { if (need_relock) arch_flash_set_wrprotect(bl, 1); - arch_flash_close(bl, NULL); + arch_flash_close(bl, flashfilename); } int main(int argc, char *argv[]) @@ -540,7 +541,6 @@ int main(int argc, char *argv[]) char *write_file = NULL, *read_file = NULL, *part_name = NULL; bool ffs_toc_seen = false, direct = false; int rc; - const char *flashfilename = NULL; while(1) { struct option long_opts[] = {
If the -F flag is used then pflash uses a regular file as the flash. On cleanup pflash fails to tell arch_flash_close() if it had passed a filename to arch_flash_init() as such arch_flash_close() assumes that it needs to close the actual flash structure and not simply a file descriptor leading to a NULL dereference. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- Note: This fix is correct but I'm not particularly fond of it. A better solution is to refactor and stop relying on atexit() to perform cleanup after the use of exit(). This cleanup would also benifit debugging, this work and tests to avoid this situation in future is in my work queue. external/pflash/pflash.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)