All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger@xenproject.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Teddy Astie <teddy.astie@vates.tech>
Subject: Re: [PATCH] x86/vRTC: don't overrun array when storing century field
Date: Mon, 7 Sep 2026 10:56:14 +0200	[thread overview]
Message-ID: <ap58LqK_m1GyrMWK@macbook.local> (raw)
In-Reply-To: <7a77f613-84b9-4049-804c-c3f53d804a38@suse.com>

On Mon, Sep 07, 2026 at 10:13:10AM +0200, Jan Beulich wrote:
> rtc_ioport_write() has two writes of the new value, yet only one was made
> aware of the century going outside of the array. Fold both writes by
> changing the RTC_SET short-circuiting.
> 
> Fixes: f2ff80877f66 ("x86/vRTC: support century field")
> Coverity ID: 1700943
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/rtc.c
> +++ b/xen/arch/x86/hvm/rtc.c
> @@ -521,20 +521,22 @@ static int rtc_ioport_write(RTCState *s,
>      case RTC_MONTH:
>      case RTC_YEAR:
>      case RTC_CENTURY:
> -        /* if in set mode, just write the register */
> -        if ( (s->hw.cmos_data[RTC_REG_B] & RTC_SET) )
> -            s->hw.cmos_data[s->hw.cmos_index] = data;
> -        else
> +        /* If in set mode, just write the register. */
> +        if ( !(s->hw.cmos_data[RTC_REG_B] & RTC_SET) )
>          {
>              /* Fetch the current time and update just this field. */
>              s->current_tm = gmtime(get_localtime(d));
>              rtc_copy_date(s);
> -            if ( s->hw.cmos_index != RTC_CENTURY )
> -                s->hw.cmos_data[s->hw.cmos_index] = data;
> -            else
> -                s->hw.century = data;
> -            rtc_set_time(s);
>          }
> +
> +        if ( s->hw.cmos_index != RTC_CENTURY )
> +            s->hw.cmos_data[s->hw.cmos_index] = data;
> +        else
> +            s->hw.century = data;

Might it be best to do this based on the array size?  ie:

if ( s->hw.cmos_index < ARRAY_SIZE(s->hw.cmos_data) )
    s->hw.cmos_data[s->hw.cmos_index] = data;
else
{
    ASSERT(s->hw.cmos_index == RTC_CENTURY);
    s->hw.century = data;
}

I don't think we are going to use more indexes, but otherwise we could
use a switch.  In any case, this is a fix so I don't intend to delay
it any longer, with either the current code or the suggested array
size checking (if suitable):

Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Roger.


  reply	other threads:[~2026-09-07  8:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07  8:13 [PATCH] x86/vRTC: don't overrun array when storing century field Jan Beulich
2026-09-07  8:56 ` Roger Pau Monné [this message]
2026-09-07  9:17   ` Jan Beulich

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=ap58LqK_m1GyrMWK@macbook.local \
    --to=roger@xenproject.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=teddy.astie@vates.tech \
    --cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.