Subject: Re: UFS quota null pointer dereference
To: None <tech-kern@netbsd.org>
From: Juergen Hannken-Illjes <hannken@eis.cs.tu-bs.de>
List: tech-kern
Date: 06/25/2007 17:35:12
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'.

-- 
Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)