On 1/28/19 12:12, Jan Beulich wrote: >>>> On 28.01.19 at 12:03, <nmant...@amazon.de> wrote: >> On 1/25/19 17:34, Jan Beulich wrote: >>>>>> On 23.01.19 at 12:57, <nmant...@amazon.de> wrote: >>>> @@ -212,7 +217,12 @@ static void vioapic_write_redirent( >>>> struct hvm_irq *hvm_irq = hvm_domain_irq(d); >>>> union vioapic_redir_entry *pent, ent; >>>> int unmasked = 0; >>>> - unsigned int gsi = vioapic->base_gsi + idx; >>>> + unsigned int gsi; >>>> + >>>> + /* Make sure no out-of-bound value for idx can be used */ >>>> + idx = array_index_nospec(idx, vioapic->nr_pins); >>>> + >>>> + gsi = vioapic->base_gsi + idx; >>> I dislike the disconnect from the respective bounds check: There's >>> only one caller, so the construct could be moved there, or >>> otherwise I'd like to see an ASSERT() added documenting that the >>> bounds check is expected to have happened in the caller. >> I agree that the idx value is used as an array index in this function >> only once. However, the gsi value also uses the value of idx, and as >> that is passed to other functions, I want to bound the gsi variable as >> well. Therefore, I chose to have a separate assignment for the idx variable. > I don't mind the separate assignment, and I didn't complain > about idx being used just once. What I said is that there's > only one caller of the function. If the bounds checking was > done there, "gsi" here would be equally "bounded" afaict. > And I did suggest an alternative in case you dislike the > moving of the construct you add.
Ah, I understand your previous sentence differently now. Thanks for clarifying. I like to keep the nospec statements close to the problematic use, so that eventual future callers benefit from that as well. Therefore, I'll add an ASSERT statement with the bound check. Best, Norbert > > Jan > > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
