svn commit: r274721 - head/sys/geom/part

[Andrey V. Elsukov]

On 20.11.2014 07:38, Bruce Evans wrote:
> On Wed, 19 Nov 2014, Warner Losh wrote:
> 
>> Log:
>>  The number of BSD partitions is variable. Return the proper number
>>  (which is in basetable->gpt_entries).
>>
>>  Submitted by: ae@
>>
>> Modified:
>>  head/sys/geom/part/g_part_bsd.c
>>
>> Modified: head/sys/geom/part/g_part_bsd.c
>> ==============================================================================
>>
>> --- head/sys/geom/part/g_part_bsd.c    Wed Nov 19 18:19:21 2014   
>> (r274720)
>> +++ head/sys/geom/part/g_part_bsd.c    Wed Nov 19 18:55:27 2014   
>> (r274721)
>> @@ -521,7 +521,7 @@ g_part_bsd_ioctl(struct g_part_table *ba
>>
>>         table = (struct g_part_bsd_table *)basetable;
>>         p = table->bbarea + pp->sectorsize;
>> -        return (bsd_disklabel_le_dec(p, data, MAXPARTITIONS));
>> +        return (bsd_disklabel_le_dec(p, data, basetable->gpt_entries));
>>     }
>>     default:
>>         return (ENOIOCTL);
> 
> How can this work?  I think you are implementing DIOCGDINFO.  This returns
> a struct disklabel, which is limited by binary compatibility to the
> historical number of partitions (8).  So this ioctl just cannot support
> more than 8 partitions.  The best that could happen is for
> bsd_disklabel_le_dec() to return an error in this case.  But it actually
> does essentially the reverse -- you pass it the maximum supported by the
> ioctl, and it returns an error if the actual number is larger.  So
> MAXPARTITIONS was correct if this is to implement DIOCGDINFO.

Yes, you are right. The size is hardcoded in the DIOCGDINFO and this can
lead to out of the bound access in the application that uses it.
Sorry for wrong suggestion.

-- 
WBR, Andrey V. Elsukov