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