public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Shi, Steven" <steven.shi@intel.com>,
	"Fu, Siyuan" <siyuan.fu@intel.com>,
	 "Ye, Ting" <ting.ye@intel.com>,
	"Wu, Jiaxin" <jiaxin.wu@intel.com>,
	"Long,  Qin" <qin.long@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH v2 4/6] MdeModulePkg/DxeNetLib: Fix negative value left shift
Date: Mon, 25 Sep 2017 06:21:39 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9796ED@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20170921064617.2628-5-hao.a.wu@intel.com>

Leave the review to module experts.

Thanks,
Star
-----Original Message-----
From: Wu, Hao A 
Sent: Thursday, September 21, 2017 2:46 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Shi, Steven <steven.shi@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Long, Qin <qin.long@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH v2 4/6] MdeModulePkg/DxeNetLib: Fix negative value left shift

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=698

Within function NetRandomInitSeed(), left shift a negative value is used
in:
"~Time.Hour << 24"

which involves undefined behavior.

Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour will be a negative value (of type int, signed).

According to the C11 spec, Section 6.5.7:
> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>   bits are filled with zeros. If E1 has an unsigned type, the value
>   of the result is E1 * 2^E2 , reduced modulo one more than the
>   maximum value representable in the result type. If E1 has a signed
>   type and nonnegative value, and E1 * 2^E2 is representable in the
>   result type, then that is the resulting value; otherwise, the
>   behavior is undefined.

This commit will remove the '~' operator before 'Time.Hour', since it seems like an implementation choice for generating the seed.

Cc: Steven Shi <steven.shi@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index 7cd7e3aca0..1ee77fe036 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -872,7 +872,7 @@ NetRandomInitSeed (
   UINT64                    MonotonicCount;
 
   gRT->GetTime (&Time, NULL);
-  Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second);
+  Seed = (Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | 
+ Time.Second);
   Seed ^= Time.Nanosecond;
   Seed ^= Time.Year << 7;
 
--
2.12.0.windows.1



  reply	other threads:[~2017-09-25  6:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-21  6:46 [PATCH v2 0/6] Resolve undefined behaviours in left shift OPs Hao Wu
2017-09-21  6:46 ` [PATCH v2 1/6] MdePkg/PrintLib: Fix possible negative value left shift Hao Wu
2017-09-28  3:58   ` Gao, Liming
2017-09-21  6:46 ` [PATCH v2 2/6] MdeModulePkg/PrintLib: " Hao Wu
2017-09-28  3:58   ` Gao, Liming
2017-09-21  6:46 ` [PATCH v2 3/6] MdeModulePkg/Tpl: Fix " Hao Wu
2017-09-25  6:21   ` Zeng, Star
2017-09-21  6:46 ` [PATCH v2 4/6] MdeModulePkg/DxeNetLib: " Hao Wu
2017-09-25  6:21   ` Zeng, Star [this message]
2017-09-21  6:46 ` [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of range " Hao Wu
2017-09-28  3:54   ` Gao, Liming
2017-09-28  3:55     ` Wu, Hao A
2017-09-21  6:46 ` [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: " Hao Wu
2017-09-25  6:24   ` Zeng, Star
2017-09-28  3:56     ` Gao, Liming
2017-09-28  5:13       ` Zeng, Star

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-list 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=0C09AFA07DD0434D9E2A0C6AEB0483103B9796ED@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /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