| Message ID | 20230403084245.54861-1-zenghao@kylinos.cn |
|---|---|
| State | New |
| Headers | show |
| Series | Subject:[PATCH] cxl-cdat:Fix open file not closed in ct3_load_cdat | expand |
On Mon, Apr 03, 2023 at 04:42:45PM +0800, zenghao wrote: > opened file processor not closed,May cause file processor leaks > > Fixes:aba578bdace5303a441f8a37aad781b5cb06f38c > > Signed-off-by: Zeng Hao <zenghao@kylinos.cn> > Suggested-by: Xie Ming <xieming@kylinos.cn> > --- > hw/cxl/cxl-cdat.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c > index 137abd0992..ba7ed1aafd 100644 > --- a/hw/cxl/cxl-cdat.c > +++ b/hw/cxl/cxl-cdat.c > @@ -128,6 +128,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) > > if (fread(cdat->buf, file_size, 1, fp) == 0) { > error_setg(errp, "CDAT: File read failed"); > + fclose(fp); > return; > } > Good catch. Reviewed-by: Fan Ni <fan.ni@samsung.com> > -- > 2.37.2 > > > No virus found > Checked by Hillstone Network AntiVirus
On Mon, 3 Apr 2023 at 13:51, zenghao <zenghao@kylinos.cn> wrote: > > opened file processor not closed,May cause file processor leaks > > Fixes:aba578bdace5303a441f8a37aad781b5cb06f38c > > Signed-off-by: Zeng Hao <zenghao@kylinos.cn> > Suggested-by: Xie Ming <xieming@kylinos.cn> > --- > hw/cxl/cxl-cdat.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c > index 137abd0992..ba7ed1aafd 100644 > --- a/hw/cxl/cxl-cdat.c > +++ b/hw/cxl/cxl-cdat.c > @@ -128,6 +128,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) > > if (fread(cdat->buf, file_size, 1, fp) == 0) { > error_setg(errp, "CDAT: File read failed"); > + fclose(fp); > return; > } Coverity also spotted this, as CID 1508069. The other bug in this code (CID 1507822) is that the check on the return value of fread() is wrong. fread() returns the number of items read or written, so checking for == 0 only catches "no data read at all", not "only read half the data". This check should be if (fread(cdat->buf, file_size, 1, fp) != file_size) { ... } I think. thanks -- PMM
On Tue, 2023-04-11 at 16:39 +0100, Peter Maydell wrote: Dear Peter Thank you for taking the time to reply to my email. I appreciate your the valuable information you have provided. > On Mon, 3 Apr 2023 at 13:51, zenghao <zenghao@kylinos.cn> wrote: > > > > opened file processor not closed,May cause file processor leaks > > > > Fixes:aba578bdace5303a441f8a37aad781b5cb06f38c > > > > Signed-off-by: Zeng Hao <zenghao@kylinos.cn> > > Suggested-by: Xie Ming <xieming@kylinos.cn> > > --- > > hw/cxl/cxl-cdat.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c > > index 137abd0992..ba7ed1aafd 100644 > > --- a/hw/cxl/cxl-cdat.c > > +++ b/hw/cxl/cxl-cdat.c > > @@ -128,6 +128,7 @@ static void ct3_load_cdat(CDATObject *cdat, > > Error **errp) > > > > if (fread(cdat->buf, file_size, 1, fp) == 0) { > > error_setg(errp, "CDAT: File read failed"); > > + fclose(fp); > > return; > > } > > Coverity also spotted this, as CID 1508069. > > The other bug in this code (CID 1507822) is that the > check on the return value of fread() is wrong. fread() > returns the number of items read or written, so > checking for == 0 only catches "no data read at all", > not "only read half the data". This check should be > if (fread(cdat->buf, file_size, 1, fp) != file_size) { > ... > } > I think. > > thanks > -- PMM I couldn't agree more with your thoughts. I will fix the bug in a separate commit.(CID 1507822) Best regards Hao
diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index 137abd0992..ba7ed1aafd 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -128,6 +128,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) if (fread(cdat->buf, file_size, 1, fp) == 0) { error_setg(errp, "CDAT: File read failed"); + fclose(fp); return; }
opened file processor not closed,May cause file processor leaks Fixes:aba578bdace5303a441f8a37aad781b5cb06f38c Signed-off-by: Zeng Hao <zenghao@kylinos.cn> Suggested-by: Xie Ming <xieming@kylinos.cn> --- hw/cxl/cxl-cdat.c | 1 + 1 file changed, 1 insertion(+)