Subject: Re: UFS quota null pointer dereference
To: None <tech-kern@netbsd.org, releng@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 06/25/2007 22:50:37
On Mon, Jun 25, 2007 at 05:35:12PM +0200, Juergen Hannken-Illjes 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'.

We may not know about all LKMs used on NetBSD...
as keeping binary compat looks easy on this one, I think we should do it for
releases.
Isn't there some userland tools using struct dquot too (quotacheck) ?

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--