On Thursday 29 September 2016 01:11 PM, Simon Horman wrote: > On Fri, Sep 23, 2016 at 03:20:46PM +0530, Pratyush Anand wrote: >> On 23/09/2016:12:47:39 PM, Madhavan Srinivasan wrote: >>> In dt_copy_old_root_param(), FILE * returned >>> from fopen is not checked for NULL pointer >>> before passinig to fclose(). This could trigger >>> a segfault. Patch adds a check. >>> >>> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com> >>> --- >>> kexec/fs2dt.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c >>> index 6ed2399759cf..2c1ebdf525f4 100644 >>> --- a/kexec/fs2dt.c >>> +++ b/kexec/fs2dt.c >>> @@ -540,7 +540,8 @@ static void dt_copy_old_root_param(void) >>> if (last_cmdline) >>> free(last_cmdline); >>> >>> - fclose(fp); >>> + if (fp) >>> + fclose(fp); >> Looks fine, however a better practise could be to return just after fopen() if >> fp was null. > That sounds like it would be somewhat cleaner. > Madhavan, could you see about making it so? Yep. re-spinning now. Maddy >