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/[email protected]/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 <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
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.
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 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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/[email protected]/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 <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> 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?
>
> 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 }
>
On Wed, 17 Aug 2022 12:30:11 +0200
Janis Schoetterl-Glausch <[email protected]> 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/[email protected]/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 <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> >
> > 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 }
> >
>
On 8/17/22 16:38, Claudio Imbrenda wrote:
> On Wed, 17 Aug 2022 12:30:11 +0200
> Janis Schoetterl-Glausch <[email protected]> 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/[email protected]/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 <[email protected]>
>>> Reported-by: Dan Carpenter <[email protected]>
>>>
>>> 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
We either have a prot value or terminate is set, right?
So I'd opt to add a PROT_NONE (with a comment that it's only used for
init). If the statement above is true we can then also add a WARN if no
valid PROT_* is set in case if terminate isn't true and a PGM_PROTECTION
has been specified.
>
>>
>>>
>>> 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 }
>>>
>>
>
On Thu, 2022-08-18 at 09:45 +0200, Janosch Frank wrote:
> On 8/17/22 16:38, Claudio Imbrenda wrote:
> > On Wed, 17 Aug 2022 12:30:11 +0200
> > Janis Schoetterl-Glausch <[email protected]> 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/[email protected]/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 <[email protected]>
> > > > Reported-by: Dan Carpenter <[email protected]>
> > > >
> > > > 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
>
> We either have a prot value or terminate is set, right?
It's independent of termination, we will have a valid value for prot if
we terminate. And I guess it still would be undefined behavior even if
the value were invalid and we terminate.
>
> So I'd opt to add a PROT_NONE (with a comment that it's only used for
> init). If the statement above is true we can then also add a WARN if no
> valid PROT_* is set in case if terminate isn't true and a PGM_PROTECTION
> has been specified.
I think what I'll do is introduce PROT_NONE, but not initialize prot
with it, but set prot to NONE whenever a code other than PGM_PROTECTION
is set.
>
>
> >
> > >
> > > >
> > > > 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 }
> > > >
> > >
> >
>