Bug 696561

Summary: Coverity scan revealed issues
Product: Red Hat Enterprise Linux 5 Reporter: Michal Luscon <mluscon>
Component: cryptsetup-luksAssignee: Milan Broz <mbroz>
Status: CLOSED ERRATA QA Contact: Release Test Team <release-test-team>
Severity: low Docs Contact:
Priority: low    
Version: 5.7CC: agk, atodorov, kdudka, mbroz, ovasik, prajnoha, prockai, pvrabec, zkabelac
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: cryptsetup-luks-1.0.3-8.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-07-21 07:04:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
Proposed solution - dereferencing null mk variable
none
Proposed solution - sector_size(fd) negative return value
none
Proposed solution - resource leak none

Description Michal Luscon 2011-04-14 10:24:34 UTC
Created attachment 492020 [details]
Proposed solution - dereferencing null mk variable

Description of problems:

1. Function "sector_size(fd)" may return negative value to variable "int bsize" which is passed to a parameter of "read" that cannot be negative in file /lib/utils.c:261.


2. In the case of return null by malloc null variable mk is dereferenced on line #49 in /luks/keymanage.c.


3. Resource leak: Function "__crypt_luks_format" in /lib/setup.c:499 returns -1 without deallocation variable mk.

Additional info:

These defects were not present in current supported version of cryptsetup-luks package.

Comment 1 Michal Luscon 2011-04-14 10:27:21 UTC
Created attachment 492021 [details]
Proposed solution - sector_size(fd) negative return value

Comment 2 Michal Luscon 2011-04-14 10:29:14 UTC
Created attachment 492024 [details]
Proposed solution - resource leak

Comment 3 Milan Broz 2011-04-14 11:54:32 UTC
cryptsetup version in RHEL5 is full of bugs which  are fixed upstream (except one). But because there was huge code changes, it cannot be easily backported.

Included patches are kind of tradeoff here, it was minimal change to fix real bugs but if RHEL management wants to take risk of replacing very uncommon fails with some possible regressions after 5.7 devel deadline, why not.

BTW one of the proposed patch does not fix the real problem (negative bsize handling in write_lseek_blockwise is wrong even upstream, you missed char frontPadBuf[bsize] on stack).

Comment 6 Ondrej Vasik 2011-04-14 12:21:01 UTC
Milan, I agree cryptsetup version in RHEL5 has plenty of bugs/defects fixed by upstream - but these defects were not present in the 5.6 version of cryptsetup - so are new in RHEL-5. I fully agree we should be very careful with fixing defects already present in RHEL-5 - it is old code and even the small change could change the behaviour.

Comment 8 Milan Broz 2011-04-14 12:47:06 UTC
(In reply to comment #6)
> Milan, I agree cryptsetup version in RHEL5 has plenty of bugs/defects fixed by
> upstream - but these defects were not present in the 5.6 version of cryptsetup
> - so are new in RHEL-5.

The new patch fixes _fatal_ buffer overflow problem for long keys (using backported patch from some of the intermediate cryptsetup version, see bug #678011). So it fixed serious bug and introduced some other (because I know what code does I can do much more better analysis than coverity which has no idea of high level code view).

I am just surprised that you are reporting it now and not before devel freeze.

Anyway, if PM is OK with fixing that, I'll do that, but there is risk of regressions.

Comment 15 Milan Broz 2011-04-19 15:11:14 UTC
Fixed in cryptsetup-luks-1.0.3-8.el5.

Comment 18 errata-xmlrpc 2011-07-21 07:04:25 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0987.html