diff mbox series

[1/1] lib/zlib: DFLTCC deflate does not write all available bits for Z_NO_FLUSH

Message ID 20230221131617.3369978-2-zaslonko@linux.ibm.com
State New
Headers
Series lib/zlib: DFLTCC deflate with Z_NO_FLUSH |

Commit Message

Zaslonko Mikhail Feb. 21, 2023, 1:16 p.m. UTC 
  DFLTCC deflate with Z_NO_FLUSH might generate a corrupted stream when
the output buffer is not large enough to fit all the deflate output at
once. The problem takes place on closing the deflate block since
flush_pending() might leave some output bits not written.
Similar problem for software deflate with Z_BLOCK flush option (not
supported by kernel zlib deflate) has been fixed a while ago in userspace
zlib but the fix never got to the kernel.
Now flush_pending() flushes the bit buffer before copying out the byte buffer,
in order to really flush as much as possible.
Currently there are no users of DFLTCC deflate with Z_NO_FLUSH option in the
kernel so the problem remained hidden for a while.

This commit is based on the old zlib commit:
https://github.com/madler/zlib/commit/0b828b4

Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 lib/zlib_deflate/defutil.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Andrew Morton Feb. 23, 2023, 8:54 p.m. UTC | #1 
On Tue, 21 Feb 2023 14:16:17 +0100 Mikhail Zaslonko <zaslonko@linux.ibm.com> wrote:

> DFLTCC deflate with Z_NO_FLUSH might generate a corrupted stream when
> the output buffer is not large enough to fit all the deflate output at
> once. The problem takes place on closing the deflate block since
> flush_pending() might leave some output bits not written.
> Similar problem for software deflate with Z_BLOCK flush option (not
> supported by kernel zlib deflate) has been fixed a while ago in userspace
> zlib but the fix never got to the kernel.
> Now flush_pending() flushes the bit buffer before copying out the byte buffer,
> in order to really flush as much as possible.
> Currently there are no users of DFLTCC deflate with Z_NO_FLUSH option in the
> kernel so the problem remained hidden for a while.
> 
> This commit is based on the old zlib commit:
> https://github.com/madler/zlib/commit/0b828b4
> 

Thanks.  Does this fix make sense when your series "lib/zlib: Set of
s390 DFLTCC related patches for kernel zlib" is not applied?  If so
then should we backport it in to earlier kernels?

If "no" then are you able to identify a suitable Fixes: tag?  So that
anyone who backports the "lib/zlib: Set of s390 DFLTCC related patches
for kernel zlib" series into an earlier kernel has a better way of
knowing that a fixup is needed.
Zaslonko Mikhail Feb. 28, 2023, 9:50 a.m. UTC | #2 
Hello Andrew,

On 23.02.2023 21:54, Andrew Morton wrote:
> On Tue, 21 Feb 2023 14:16:17 +0100 Mikhail Zaslonko <zaslonko@linux.ibm.com> wrote:
> 
>> DFLTCC deflate with Z_NO_FLUSH might generate a corrupted stream when
>> the output buffer is not large enough to fit all the deflate output at
>> once. The problem takes place on closing the deflate block since
>> flush_pending() might leave some output bits not written.
>> Similar problem for software deflate with Z_BLOCK flush option (not
>> supported by kernel zlib deflate) has been fixed a while ago in userspace
>> zlib but the fix never got to the kernel.
>> Now flush_pending() flushes the bit buffer before copying out the byte buffer,
>> in order to really flush as much as possible.
>> Currently there are no users of DFLTCC deflate with Z_NO_FLUSH option in the
>> kernel so the problem remained hidden for a while.
>>
>> This commit is based on the old zlib commit:
>> https://github.com/madler/zlib/commit/0b828b4
>>
> 
> Thanks.  Does this fix make sense when your series "lib/zlib: Set of
> s390 DFLTCC related patches for kernel zlib" is not applied?  If so
> then should we backport it in to earlier kernels?

This fix is separate and does not require a previous zlib patch series.
Although there are no users of s390 hardware deflate with Z_NO_FLUSH at the moment, 
we might still backport this fix to avoid potential issues in the future.
For all my zlib patches the following tag can be used:
  Fixes: aa5b395b69b6 ("lib/zlib: add s390 hardware support for kernel zlib_deflate")

> 
> If "no" then are you able to identify a suitable Fixes: tag?  So that
> anyone who backports the "lib/zlib: Set of s390 DFLTCC related patches
> for kernel zlib" series into an earlier kernel has a better way of
> knowing that a fixup is needed.
> 

Thanks,
Mikhail
diff mbox series

Patch

diff --git a/lib/zlib_deflate/defutil.h b/lib/zlib_deflate/defutil.h
index 385333b22ec6..4ea40f5a279f 100644
--- a/lib/zlib_deflate/defutil.h
+++ b/lib/zlib_deflate/defutil.h
@@ -420,9 +420,11 @@  static inline void flush_pending(
 	z_streamp strm
 )
 {
+    unsigned len;
     deflate_state *s = (deflate_state *) strm->state;
-    unsigned len = s->pending;
 
+    bi_flush(s);
+    len = s->pending;
     if (len > strm->avail_out) len = strm->avail_out;
     if (len == 0) return;