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 14:30:02
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