Subject: Re: UFS quota null pointer dereference
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-kern
Date: 06/25/2007 22:34:29
In article <20070625153512.GA21802@bseis.eis.cs.tu-bs.de>,
Juergen Hannken-Illjes <hannken@eis.cs.tu-bs.de> wrote:
>On Mon, Jun 25, 2007 at 02:30:02PM +0000, Christos Zoulas wrote:
>> In article <20070625140234.GA18006@bseis.eis.cs.tu-bs.de>,
>> Juergen Hannken-Illjes <hannken@eis.cs.tu-bs.de> wrote:
>> >In -current we had `struct dquot' in file sys/ufs/ufs/quota.h as:
>> >
>> >struct dquot {
>> > LIST_ENTRY(dquot) dq_hash; /* hash list */
>> > TAILQ_ENTRY(dquot) dq_freelist; /* free list */
>> > u_int16_t dq_flags; /* flags, see below */
>> > u_int16_t dq_cnt; /* count of active references */
>> > u_int16_t dq_spare; /* unused spare padding */
>> > u_int16_t dq_type; /* quota type of this dquot */
>> > u_int32_t dq_id; /* identifier this applies to */
>> > struct ufsmount *dq_ump; /* filesystem that this is
>taken from */
>> > struct dqblk dq_dqb; /* actual usage & quotas */
>> >};
>> >
>> >This leads to a null pointer derefence if a quota-enabled file system
>> >has 65536 active vnodes for one uid becaus `dq_cnt' overflows.
>> >
>> >So in -current I changed the type of `dq_cnt' to `u_int32_t'.
>> >Unfortunately this cannot be pulled up to netbsd-2-x and netbsd-3-x
>> >because it breaks backwards compatibility with 3rd party LKM's.
>> >
>> >A quick hack would be to split the counter into two fields:
>> >
>> > u_int16_t dq_cnt; /* count of active references low */
>> >- u_int16_t dq_spare; /* unused spare padding */
>> >+ u_int16_t dq_cnt_upper; /* count of active references high */
>> >
>> >and change sys/ufs/ufs/ufs_quota.c to care for this split counter.
>> >
>> >Opinions?
>>
>> Is the LKM compatibility issue real? I.e. are you aware of any lkm that
>> depends on dq_cnt being 16 bit, or even an lkm that uses struct dquot?
>> If there isn't, I think that it is pretty safe to make it a 32 bit number
>> and pull it up as such.
>>
>> christos
>
>I'm not aware of any such lkm. `struct dquot' and its consumers in
>ufs_quota.c should be private but unfortunately they are exported via
>`quota.h'.
Just make the change then and put a note in the release notes for good
measure.
christos