From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 3/5] n16550: add sanity check for reg_shift Date: Tue, 19 Jan 2016 06:32:19 -0700 Message-ID: <569E48F302000078000C88E6@prv-mh.provo.novell.com> References: <568CEBD002000078000C3D17@prv-mh.provo.novell.com> <1453183077-50542-1-git-send-email-czylin@uwaterloo.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1453183077-50542-1-git-send-email-czylin@uwaterloo.ca> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chester Lin Cc: ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, george.dunlap@eu.citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jtotto@uwaterloo.ca, hjarmstr@uwaterloo.ca List-Id: xen-devel@lists.xenproject.org >>> On 19.01.16 at 06:57, wrote: > Fix CID 1343302 by adding checking a check on the value of reg_shift. > This patch also rolls the multiplication by 8 into the shift. > No functional changes. > > Suggested-by: Jan Beulich I don't think so. > Signed-off-by: Chester Lin > --- > xen/drivers/char/ns16550.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index bc24015..55cfc45 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -913,7 +913,8 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int bar_idx) > * Force length of mmio region to be at least > * 8 bytes times (1 << reg_shift) > */ > - if ( size < (0x8 * (1 << uart_param[p].reg_shift)) ) > + if ( uart_param[p].reg_shift > 27 || > + size < (1 << (uart_param[p].reg_shift + 3)) ) > continue; Instead I think Coverity complaining is mad, and adding a comparison here just clutters the code. The only thing I could imagine I might have suggested would be to put an ASSERT() here. In any event should is the replacement of the multiplication by an addition not what I think I had also mentioned before: The expression, if changed in the first place, should use 8 as the left operand of the shift. Jan