[systemd-devel] [PATCH] cryptsetup: check that password is not null

Thomas H.P. Andersen phomes at gmail.com
Thu Jun 12 15:31:51 PDT 2014


On Fri, Jun 13, 2014 at 12:01 AM, Greg KH <gregkh at linuxfoundation.org> wrote:
> On Thu, Jun 12, 2014 at 11:49:35PM +0200, Thomas H.P. Andersen wrote:
>> On Thu, Jun 12, 2014 at 11:08 PM, Greg KH <gregkh at linuxfoundation.org> wrote:
>> > On Thu, Jun 12, 2014 at 10:55:50PM +0200, Thomas H.P. Andersen wrote:
>> >> From: Thomas Hindoe Paaboel Andersen <phomes at gmail.com>
>> >>
>> >> Beef up the assert to protect against passing null to strlen.
>> >>
>> >> Found with scan-build.
>> >> ---
>> >>  src/cryptsetup/cryptsetup.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
>> >> index 812b32f..a67d85e 100644
>> >> --- a/src/cryptsetup/cryptsetup.c
>> >> +++ b/src/cryptsetup/cryptsetup.c
>> >> @@ -344,7 +344,7 @@ static int attach_tcrypt(struct crypt_device *cd,
>> >>
>> >>          assert(cd);
>> >>          assert(name);
>> >> -        assert(key_file || passwords);
>> >> +        assert(key_file || (passwords && passwords[0]));
>> >
>> > Shouldn't strlen of an "empty" string just return 0?
>>
>> Passing null to strlen is undefined behavior and seg faults reliably for me.
>
> NULL, yes, but that's already checked.  Passing a string that is "\0"
> should work properly, which is all you added the test for here, right?
>
> Oh doh, that's an array of an array, nevermind, the patch is correct,
> my fault.

Thanks. Applied.


More information about the systemd-devel mailing list