linux-kernel - [BUG] arch/x86/kvm/x86.c: In function ‘prepare_emulation_failure_exit’: error: use of NULL ‘data’ where non-null expected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1eb96f85-edee-45fc-930f-a192cecbf54c@gmail.com>
Date: Fri, 19 Jul 2024 20:22:05 +0200
From: Mirsad Todorovac <mtodorovac69@...il.com>
To: kvm@...r.kernel.org
Cc: Sean Christopherson <seanjc@...gle.com>,
 Paolo Bonzini <pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
 David Edmondson <david.edmondson@...cle.com>
Subject: [BUG] arch/x86/kvm/x86.c: In function ‘prepare_emulation_failure_exit’: error: use of NULL ‘data’ where non-null expected

Hi, all!

On linux-stable 6.10 vanilla tree, another NULL pointer is passed, which was detected
by the fortify-string.h mechanism.

arch/x86/kvm/x86.c
==================

13667 kvm_prepare_emulation_failure_exit(vcpu);

calls

8796 __kvm_prepare_emulation_failure_exit(vcpu, NULL, 0);

which calls

8790 prepare_emulation_failure_exit(vcpu, data, ndata, NULL, 0);

Note here that data == NULL and ndata = 0.

again data == NULL and ndata == 0, which passes unchanged all until

8773 memcpy(&run->internal.data[info_start + ARRAY_SIZE(info)], data, ndata * sizeof(data[0]));

The problem code was introduced with the commit e615e355894e6.

arch/x86/kvm/x86.c
==================
  8728 static void prepare_emulation_failure_exit(struct kvm_vcpu *vcpu, u64 *data,
  8729                                            u8 ndata, u8 *insn_bytes, u8 insn_size)
  8730 {
  8731         struct kvm_run *run = vcpu->run;
  8732         u64 info[5];
  8733         u8 info_start;
  8734 
  8735         /*
  8736          * Zero the whole array used to retrieve the exit info, as casting to
  8737          * u32 for select entries will leave some chunks uninitialized.
  8738          */
  8739         memset(&info, 0, sizeof(info));
  8740 
  8741         static_call(kvm_x86_get_exit_info)(vcpu, (u32 *)&info[0], &info[1],
  8742                                            &info[2], (u32 *)&info[3],
  8743                                            (u32 *)&info[4]);
  8744         
  8745         run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
  8746         run->emulation_failure.suberror = KVM_INTERNAL_ERROR_EMULATION;
  8747 
  8748         /*
  8749          * There's currently space for 13 entries, but 5 are used for the exit
  8750          * reason and info.  Restrict to 4 to reduce the maintenance burden
  8751          * when expanding kvm_run.emulation_failure in the future.
  8752          */
  8753         if (WARN_ON_ONCE(ndata > 4))
  8754                 ndata = 4;
  8755 
  8756         /* Always include the flags as a 'data' entry. */
  8757         info_start = 1;
  8758         run->emulation_failure.flags = 0;
  8759         
  8760         if (insn_size) {
  8761                 BUILD_BUG_ON((sizeof(run->emulation_failure.insn_size) +
  8762                               sizeof(run->emulation_failure.insn_bytes) != 16));
  8763                 info_start += 2;
  8764                 run->emulation_failure.flags |=
  8765                         KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES;
  8766                 run->emulation_failure.insn_size = insn_size;
  8767                 memset(run->emulation_failure.insn_bytes, 0x90,
  8768                        sizeof(run->emulation_failure.insn_bytes));
  8769                 memcpy(run->emulation_failure.insn_bytes, insn_bytes, insn_size);
  8770         }
  8771         
  8772         memcpy(&run->internal.data[info_start], info, sizeof(info));
  8773         memcpy(&run->internal.data[info_start + ARRAY_SIZE(info)], data,
  8774                ndata * sizeof(data[0]));   
  8775  
  8776         run->emulation_failure.ndata = info_start + ARRAY_SIZE(info) + ndata;
  8777 }               
  8778 
  8779 static void prepare_emulation_ctxt_failure_exit(struct kvm_vcpu *vcpu)
  8780 {
  8781         struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
  8782                 
  8783         prepare_emulation_failure_exit(vcpu, NULL, 0, ctxt->fetch.data,
  8784                                        ctxt->fetch.end - ctxt->fetch.data);
  8785 }
  8786                 
  8787 void __kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu, u64 *data,
  8788                                           u8 ndata)
  8789 {
  8790         prepare_emulation_failure_exit(vcpu, data, ndata, NULL, 0);
  8791 }
  8792 EXPORT_SYMBOL_GPL(__kvm_prepare_emulation_failure_exit);
  8793 
  8794 void kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu)
  8795 {
  8796         __kvm_prepare_emulation_failure_exit(vcpu, NULL, 0);
  8797 }

Probably before memcpy() with ndata == 0 ended in a NOOP, but now CONFIG_FORTIFY_SOURCE=y
turns a warning, or prevents the build with -Werror.

Hope this helps.

Best regards,
Mirsad Todorovac