[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] fix misc issues related to allowingsupport of more CPUs

  • To: Jan Beulich <jbeulich@xxxxxxxxxx>
  • From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
  • Date: Mon, 22 Sep 2008 15:02:39 +0100
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 22 Sep 2008 07:03:06 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AckcjiZIZLxHkIiBEd2psAAX8io7RQALbkrY
  • Thread-topic: [Xen-devel] [PATCH] fix misc issues related to allowingsupport of more CPUs

On 22/9/08 09:35, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote:

> On 22/9/08 08:46, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
>>> Why must it be WARN_ON()? You think you could specify strings so long that
>>> they overflow 32 bits? You've got other problems in that case.
>> No, that's not the reason. It's because of how bitmap_scnprintf() and
>> bitmap_scnlistprintf() work - they can (validly, assuming that the code
>> having been derived from Linux and still being that way in Linux, hence
>> apparently considered correct) pass negative sizes to scnprintf(), and
>> hence it must not kill the system to actually do so.
> The obvious answer would be to fix the bogus callers. Or consider negative
> size to be a valid input. Warning on what callers consider valid behaviour
> is just weird. I would say the former (fix the callers) is the better way to
> go here; presumably they just need to clamp the size parameter.

Further to this, actually I think the callers in bitmap.c are correct.
Assuming len<=buflen at the start of the bitmap_scn*() functions (valid
since buflen is non-negative and len == 0 at function start) then we'll
never have len>buflen because scnprintf() truncates its return value to be
less than its size parameter, which is always buflen-len.

What *is* a bug is that scnprintf() returns -1 when passed a size==0, which
it specifically is not supposed to do! I notice this bug appears to live on
in current Linux 2.6 too.

I will fix this bug and extend the vsnprintf() BUG_ON() with the predicate
you used for your WARN_ON() logic. Beyond that I don't think anything needs
to be done.

 -- Keir

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.