arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
@ 2022-08-15  7:23 Dan Carpenter
  2022-08-17 10:30 ` Janis Schoetterl-Glausch
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-08-15  7:23 UTC (permalink / raw)
  To: kbuild, Janis Schoetterl-Glausch
  Cc: lkp, kbuild-all, linux-kernel, Janosch Frank, Claudio Imbrenda

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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.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.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
  2022-08-15  7:23 arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot' Dan Carpenter
@ 2022-08-17 10:30 ` Janis Schoetterl-Glausch
  2022-08-17 14:38   ` Claudio Imbrenda
  0 siblings, 1 reply; 5+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-08-17 10:30 UTC (permalink / raw)
  To: Dan Carpenter, kbuild
  Cc: lkp, kbuild-all, linux-kernel, Janosch Frank, Claudio Imbrenda

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@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.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?

> 
> 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  }
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
  2022-08-17 10:30 ` Janis Schoetterl-Glausch
@ 2022-08-17 14:38   ` Claudio Imbrenda
  2022-08-18  7:45     ` Janosch Frank
  0 siblings, 1 reply; 5+ messages in thread
From: Claudio Imbrenda @ 2022-08-17 14:38 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch
  Cc: Dan Carpenter, kbuild, lkp, kbuild-all, linux-kernel, Janosch Frank

On Wed, 17 Aug 2022 12:30:11 +0200
Janis Schoetterl-Glausch <scgl@linux.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@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.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  }
> >   
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
  2022-08-17 14:38   ` Claudio Imbrenda
@ 2022-08-18  7:45     ` Janosch Frank
  2022-08-18 13:14       ` Janis Schoetterl-Glausch
  0 siblings, 1 reply; 5+ messages in thread
From: Janosch Frank @ 2022-08-18  7:45 UTC (permalink / raw)
  To: Claudio Imbrenda, Janis Schoetterl-Glausch
  Cc: Dan Carpenter, kbuild, lkp, kbuild-all, linux-kernel

On 8/17/22 16:38, Claudio Imbrenda wrote:
> On Wed, 17 Aug 2022 12:30:11 +0200
> Janis Schoetterl-Glausch <scgl@linux.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@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.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

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  }
>>>    
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'.
  2022-08-18  7:45     ` Janosch Frank
@ 2022-08-18 13:14       ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 5+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-08-18 13:14 UTC (permalink / raw)
  To: Janosch Frank, Claudio Imbrenda
  Cc: Dan Carpenter, kbuild, lkp, kbuild-all, linux-kernel

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 <scgl@linux.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@intel.com>
> > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.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
> 
> 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  }
> > > >    
> > > 
> > 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-08-18 13:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15  7:23 arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot' Dan Carpenter
2022-08-17 10:30 ` Janis Schoetterl-Glausch
2022-08-17 14:38   ` Claudio Imbrenda
2022-08-18  7:45     ` Janosch Frank
2022-08-18 13:14       ` Janis Schoetterl-Glausch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®