[bug report] erofs: enable error reporting for z_erofs_fixup_insize()

[Gao Xiang]

On 2025/12/2 16:43, Gao Xiang wrote:
> Hi Dan,
> 
> On 2025/12/2 16:32, Dan Carpenter wrote:
>> Hello Gao Xiang,
>>
>> Commit 30e13e41a0eb ("erofs: enable error reporting for
>> z_erofs_fixup_insize()") from Nov 27, 2025 (linux-next), leads to the
>> following Smatch static checker warning:
>>
>> fs/erofs/decompressor.c:217 z_erofs_lz4_decompress_mem() warn: 'reason' isn't an ERR_PTR
>> fs/erofs/decompressor_lzma.c:193 z_erofs_lzma_decompress() warn: 'reason' is an error pointer or valid
>> fs/erofs/decompressor_zstd.c:180 z_erofs_zstd_decompress() warn: 'reason' is an error pointer or valid

BTW, I don't think any issue out of `fs/erofs/decompressor_lzma.c`
and `fs/erofs/decompressor_zstd.c`.

"reason != NULL" means failure, so it will break and propagate to
the caller.

>>
>> fs/erofs/decompressor.c
>>      198 static int z_erofs_lz4_decompress_mem(struct z_erofs_decompress_req *rq, u8 *dst)
>>      199 {
>>      200         bool support_0padding = false, may_inplace = false;
>>      201         unsigned int inputmargin;
>>      202         u8 *out, *headpage, *src;
>>      203         const char *reason;
>>      204         int ret, maptype;
>>      205
>>      206         DBG_BUGON(*rq->in == NULL);
>>      207         headpage = kmap_local_page(*rq->in);
>>      208
>>      209         /* LZ4 decompression inplace is only safe if zero_padding is enabled */
>>      210         if (erofs_sb_has_zero_padding(EROFS_SB(rq->sb))) {
>>      211                 support_0padding = true;
>>      212                 reason = z_erofs_fixup_insize(rq, headpage + rq->pageofs_in,
>>      213                                 min_t(unsigned int, rq->inputsize,
>>      214                                       rq->sb->s_blocksize - rq->pageofs_in));
>>      215                 if (reason) {
>>      216                         kunmap_local(headpage);
>> --> 217                         return IS_ERR(reason) ? PTR_ERR(reason) : -EFSCORRUPTED;
>>      218                 }
>>
>> The z_erofs_fixup_insize() function used to return error pointers, but
>> now it returns an error string or NULL.  So probably we could just
>> change this to:
>>
>>         return -EFSCORRUPTED;
> 
> I think `return -EFSCORRUPTED;` is valid for now, but later
> z_erofs_lz4_decompress_mem() will be changed into returning
> `const char *` as well but it's late for this cycle.
> 
>>
>> Are we planning to make it return error pointers again?
> 
> z_erofs_lz4_decompress_mem() will be `const char *` later too
> along with other cleanups, but it's somewhat late for this cycle.
> 
>>
>> NULL means success in this case, right?  It's sort of weird how NULL means
>> success and a valid pointer means failure.
> 
> Because -EFSCORRUPTED isn't enough to represent the detailed
> decompression errors (because each decompressor has its own
> error messages and it's helpful for users to know how the
> compressed data is corrupted).
> 
> NULL means success, and both valid pointers or error pointers
> != NULL means failure, so personally I don't think it's weird.
> 
> A valid pointer indicates an detailed error string to be
> printed so that users know the detailed reasons and the error
> number is -EFSCORRUPTED.
> 
> Thanks,
> Gao Xiang