Re: New Defects reported by Coverity Scan for XenProject

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.05.2024 11:54, scan-admin@xxxxxxxxxxxx wrote:
> Hi,
> 
> Please find the latest report on new defect(s) introduced to XenProject found 
> with Coverity Scan.
> 
> 2 new defect(s) introduced to XenProject found with Coverity Scan.
> 1 defect(s), reported by Coverity Scan earlier, were marked fixed in the 
> recent build analyzed by Coverity Scan.
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 2 of 2 defect(s)
> 
> 
> ** CID 1596837:    (USE_AFTER_FREE)
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> /xen/common/gzip/inflate.c: 935 in inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 935 in 
> inflate_dynamic()
> /xen/common/gzip/inflate.c: 935 in inflate_dynamic()
> /xen/common/gzip/inflate.c: 935 in inflate_dynamic()
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 935 in 
> inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> 
> 
> ________________________________________________________________________________________________________
> *** CID 1596837:    (USE_AFTER_FREE)
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> 937             goto out;
> 938         }
> 939     
> 940         DEBG("dyn6 ");
> 941     
> 942         /* decompress until an end-of-block code */
>>>>     CID 1596837:    (USE_AFTER_FREE)
>>>>     Calling "inflate_codes" dereferences freed pointer "tl".
> 943         if (inflate_codes(tl, td, bl, bd)) {
> 944             ret = 1;
> 945             goto out;
> 946         }

While first I thought the tool may be confused by the earlier huft_free()
(matching an earlier huft_build()), ...

> ** CID 1596836:    (USE_AFTER_FREE)
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> /tools/firmware/xen-dir/xen-root/xen/common/gzip/inflate.c: 943 in 
> inflate_dynamic()
> 
> 
> ________________________________________________________________________________________________________
> *** CID 1596836:    (USE_AFTER_FREE)
> /xen/common/gzip/inflate.c: 943 in inflate_dynamic()
> 937             goto out;
> 938         }
> 939     
> 940         DEBG("dyn6 ");
> 941     
> 942         /* decompress until an end-of-block code */
>>>>     CID 1596836:    (USE_AFTER_FREE)
>>>>     Calling "inflate_codes" dereferences freed pointer "td".
> 943         if (inflate_codes(tl, td, bl, bd)) {
> 944             ret = 1;
> 945             goto out;
> 946         }

... no dual usage exists for td. Hence I'm utterly confused as to what the
tool is "thinking". In fact it looks like there is an opposite issue in
both inflate_fixed() and inflate_dynamic(): tl and td are leaked when
inflate_codes() fails. I guess I'll make a patch ...

Jan