Re: [PATCH] KVM: arm64: its: Fix missing dynamic allocation check in scan_its_table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 13/10/17 12:57, Christoffer Dall wrote:
> On Fri, Oct 13, 2017 at 12:00:50PM +0100, Marc Zyngier wrote:
>> On 13/10/17 11:52, Christoffer Dall wrote:
>>> We currently allocate an entry dynamically, but we never check if the
>>> allocation actually succeeded.  We actually don't need a dynamic
>>> allocation, because we know the maximum size of an ITS table entry, so
>>> we can simply use an allocation on the stack.
>>>
>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
>>> ---
>>>  virt/kvm/arm/vgic/vgic-its.c | 19 ++++++++-----------
>>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>>> index f51c1e1..555f42f 100644
>>> --- a/virt/kvm/arm/vgic/vgic-its.c
>>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>>> @@ -176,6 +176,7 @@ static const struct vgic_its_abi its_table_abi_versions[] = {
>>>  };
>>>  
>>>  #define NR_ITS_ABIS	ARRAY_SIZE(its_table_abi_versions)
>>> +#define MAX_ENTRY_SIZE	8	/* Max Entry size across all ABI versions */
>>>  
>>>  inline const struct vgic_its_abi *vgic_its_get_abi(struct vgic_its *its)
>>>  {
>>> @@ -1801,37 +1802,33 @@ typedef int (*entry_fn_t)(struct vgic_its *its, u32 id, void *entry,
>>>  static int scan_its_table(struct vgic_its *its, gpa_t base, int size, int esz,
>>>  			  int start_id, entry_fn_t fn, void *opaque)
>>>  {
>>> -	void *entry = kzalloc(esz, GFP_KERNEL);
>>>  	struct kvm *kvm = its->dev->kvm;
>>>  	unsigned long len = size;
>>>  	int id = start_id;
>>>  	gpa_t gpa = base;
>>> +	char entry[MAX_ENTRY_SIZE];
>>
>> Nit: you can drop the memset below if you initialize immediately, and
>> GCC supporting dynamic arrays allows you to write this:
>>
>> 	char entry[esz] = { 0, };
>>
>> so that you don't have to add the MAX_ENTRY_SIZE.
>>
> 
> Using a dynamic sized array is a great idea, but trying to initalize it
> at the same time gives me:
> 
>     error: variable-sized object may not be initialized
> 
> Is there a trick I'm unfamiliar with?

No, it is just that these two things are mutually incompatible, and I
didn't realize it. You have to choose one or the other... :-(

I suggest you keep the memset...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]