From: Rusty Russell <rusty@rustcorp.com.au>
To: "Timothy B. Terriberry" <tterribe@xiph.org>,
David Gibson <david@gibson.dropbear.id.au>
Cc: ccan@lists.ozlabs.org
Subject: Re: [PATCH 4/7] crypto/hmac_sha256: Remove undefined memset()
Date: Mon, 24 Apr 2017 12:00:15 +0930 [thread overview]
Message-ID: <87tw5efsg8.fsf@rustcorp.com.au> (raw)
In-Reply-To: <58F59470.4070507@xiph.org>
"Timothy B. Terriberry" <tterribe@xiph.org> writes:
> Rusty Russell wrote:
>>> Well.. more precisely, my understanding is that memset(p, x, 0) can't
>>> be counted on not to dereference p.
>>
>> That seems nonsensical, though.
>
> It is nonsensical, but welcome to standards.
>
> C99 Section 7.21.1 "String function conventions" (which includes memset,
> as it is in string.h):
>
> "Where an argument declared as size_t n specifies the length of the
> array for a function, n can have the value zero on a call to that
> function. Unless explicitly stated otherwise in the description of a
> particular function in this subclause, pointer arguments on such a call
> shall still have valid values, as described in 7.1.4"
I think you're confusing "valid pointer" with "dereferencable pointer".
A pointer one past the end is still a valid pointer, just not
dereferencable. In this case, that's what we have when ksize ==
HMAC_SHA256_BLOCKSIZE:
memset((char *)k_ipad + ksize, 0, HMAC_SHA256_BLOCKSIZE - ksize);
ie. we are trying to set the unused trailing bytes of the region to
zero.
Even if you were correct, and code should be testing for zero sizes in
such cases, and don't care that Kernighan and Plauger would be rolling
in their graves (and neither is even dead yet!), you would just cause me
to write a test case.
Because you should *never* give in to insanity except under duress, and
then with loud protest! Otherwise everyone loses.
Cheers,
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan
next prev parent reply other threads:[~2017-04-24 2:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-03 11:11 [PATCH 0/7] Coverity David Gibson
2017-04-03 11:11 ` [PATCH 1/7] .travis.yml: Add support for Coverity Scan David Gibson
2017-04-03 12:14 ` David Gibson
2017-04-03 11:11 ` [PATCH 2/7] failtest: Remove memory leak David Gibson
2017-04-03 11:11 ` [PATCH 3/7] tools: Remove fd leak David Gibson
2017-04-03 11:11 ` [PATCH 4/7] crypto/hmac_sha256: Remove undefined memset() David Gibson
2017-04-04 2:22 ` Rusty Russell
2017-04-05 12:23 ` David Gibson
2017-04-12 23:56 ` Rusty Russell
2017-04-18 4:22 ` Timothy B. Terriberry
2017-04-24 2:30 ` Rusty Russell [this message]
2017-04-03 11:11 ` [PATCH 5/7] crypto/ripemd160: Correct badly sized union member David Gibson
2017-04-03 11:11 ` [PATCH 6/7] ccanlint: Fix leak in do_reduce_features() David Gibson
2017-04-03 11:11 ` [PATCH 7/7] net: Add check for failure of setsockopt() David Gibson
2017-04-04 2:23 ` Rusty Russell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tw5efsg8.fsf@rustcorp.com.au \
--to=rusty@rustcorp.com.au \
--cc=ccan@lists.ozlabs.org \
--cc=david@gibson.dropbear.id.au \
--cc=tterribe@xiph.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).