On 6 August 2012 08:36, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> +static int copy_reg_to_user(const struct coproc_reg *reg, u32 __user **uind)
> +{
> + if (!*uind)
> + return 0;
> +
> + if (put_user(cp15_to_index(reg), *uind))
> + return -EFAULT;
> +
> + (*uind)++;
> + return 0;
> +}
This function returns 0 on success or a negative errno on failure...
> +
> +/* Assumed ordered tables, see kvm_coproc_table_init. */
> +static int walk_msrs(struct kvm_vcpu *vcpu, u32 __user *uind)
> +{
> + const struct coproc_reg *i1, *i2, *end1, *end2;
> + unsigned int total = 0;
> + size_t num;
> +
> + /* We check for duplicates here, to allow arch-specific overrides. */
> + i1 = get_target_table(vcpu->arch.target, &num);
> + end1 = i1 + num;
> + i2 = cp15_regs;
> + end2 = cp15_regs + ARRAY_SIZE(cp15_regs);
> +
> + BUG_ON(i1 == end1 || i2 == end2);
> +
> + /* Walk carefully, as both tables may refer to the same register. */
> + while (i1 && i2) {
> + int cmp = cmp_reg(i1, i2);
> + /* target-specific overrides generic entry. */
> + if (cmp <= 0) {
> + /* Ignore registers we trap but don't save. */
> + if (i1->reg) {
> + if (!copy_reg_to_user(i1, &uind))
> + return -EFAULT;
...but this code assumes it returns 0 on failure
> + total++;
> + }
> + } else {
> + /* Ignore registers we trap but don't save. */
> + if (i2->reg) {
> + if (!copy_reg_to_user(i2, &uind))
> + return -EFAULT;
...as does this.
The effect is that
(a) trying to query the length of the list returns a too small
or even negative number since kvm_num_guest_msrs() doesn't notice
it just got handed an errno
(b) trying to actually read the list always fails -EFAULT.
Dropping the "!"s fixes this for me, but you might prefer to
change the return value of copy_reg_to_user() instead.
-- PMM
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
