| 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
| ||
|
Date: Wed, 17 Aug 2022 16:38:38 +0200
From: Claudio Imbrenda <imbrenda@...ux.ibm.com>
To: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
Cc: Dan Carpenter <dan.carpenter@...cle.com>, kbuild@...ts.01.org,
lkp@...el.com, kbuild-all@...ts.01.org,
linux-kernel@...r.kernel.org, Janosch Frank <frankja@...ux.ibm.com>
Subject: Re: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error:
uninitialized symbol 'prot'.
On Wed, 17 Aug 2022 12:30:11 +0200
Janis Schoetterl-Glausch <scgl@...ux.ibm.com> wrote:
> On 8/15/22 09:23, Dan Carpenter wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 69dac8e431af26173ca0a1ebc87054e01c585bcc
> > commit: 7faa543df19bf62d4583a64d3902705747f2ad29 KVM: s390: gaccess: Refactor access address range check
> > config: s390-randconfig-m031-20220812 (https://download.01.org/0day-ci/archive/20220814/202208140141.4uzWszG4-lkp@intel.com/config)
> > compiler: s390-linux-gcc (GCC) 12.1.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@...el.com>
> > Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
> >
> > smatch warnings:
> > arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
> >
> > vim +/prot +859 arch/s390/kvm/gaccess.c
> >
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 831 static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 832 unsigned long *gpas, unsigned long len,
> > 92c9632119b67f David Hildenbrand 2015-11-16 833 const union asce asce, enum gacc_mode mode)
> > 2293897805c2fe Heiko Carstens 2014-01-01 834 {
> > 2293897805c2fe Heiko Carstens 2014-01-01 835 psw_t *psw = &vcpu->arch.sie_block->gpsw;
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 836 unsigned int offset = offset_in_page(ga);
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 837 unsigned int fragment_len;
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 838 int lap_enabled, rc = 0;
> > 6ae1574c2a24ee Christian Borntraeger 2017-06-07 839 enum prot_type prot;
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 840 unsigned long gpa;
> > 2293897805c2fe Heiko Carstens 2014-01-01 841
> > 75a1812230ad7a Alexander Yarygin 2015-01-22 842 lap_enabled = low_address_protection_enabled(vcpu, asce);
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 843 while (min(PAGE_SIZE - offset, len) > 0) {
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 844 fragment_len = min(PAGE_SIZE - offset, len);
> > 2293897805c2fe Heiko Carstens 2014-01-01 845 ga = kvm_s390_logical_to_effective(vcpu, ga);
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 846 if (mode == GACC_STORE && lap_enabled && is_low_address(ga))
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 847 return trans_exc(vcpu, PGM_PROTECTION, ga, ar, mode,
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 848 PROT_TYPE_LA);
> > a752598254016d Heiko Carstens 2017-06-03 849 if (psw_bits(*psw).dat) {
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 850 rc = guest_translate(vcpu, ga, &gpa, asce, mode, &prot);
> > 2293897805c2fe Heiko Carstens 2014-01-01 851 if (rc < 0)
> > 2293897805c2fe Heiko Carstens 2014-01-01 852 return rc;
> > 2293897805c2fe Heiko Carstens 2014-01-01 853 } else {
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 854 gpa = kvm_s390_real_to_abs(vcpu, ga);
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 855 if (kvm_is_error_gpa(vcpu->kvm, gpa))
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 856 rc = PGM_ADDRESSING;
> > ^^^^^^^^^^^^^^^^^^^^
> >
> > "rc" set but "prot" not initialized.
>
> prot is only used in case of PGM_PROTECTION.
> Because of that, we could initialize prot to an arbitrary value, but that seems misleading and a bit ugly to me.
> Alternatively, we could introduce a PROT_NONE.
> Or we do nothing, since there is no actual problem.
> @Janosch, @Claudio, what do you think?
I agree that this is not a bug.
It does look ugly, though, and all reasonable solutions are also ugly
(each for a different reason).
another solution is to set prot to an arbitrary value only in the if
case marked above. that way prot is not arbitrarily initialized, and
there is no need to add a new PROT_NONE (which then would need to be
checked for in trans_exc)
I do not have a strong opinion, though
>
> >
> > 2293897805c2fe Heiko Carstens 2014-01-01 857 }
> > cde0dcfb5df1db David Hildenbrand 2016-05-31 858 if (rc)
> > 6ae1574c2a24ee Christian Borntraeger 2017-06-07 @859 return trans_exc(vcpu, rc, ga, ar, mode, prot);
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 860 if (gpas)
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 861 *gpas++ = gpa;
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 862 offset = 0;
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 863 ga += fragment_len;
> > 7faa543df19bf6 Janis Schoetterl-Glausch 2021-11-26 864 len -= fragment_len;
> > 2293897805c2fe Heiko Carstens 2014-01-01 865 }
> > 2293897805c2fe Heiko Carstens 2014-01-01 866 return 0;
> > 2293897805c2fe Heiko Carstens 2014-01-01 867 }
> >
>
Powered by blists - more mailing lists