Subject: Re: kern/21474
To: None <peter@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: Peter Postma <peter@pointless.nl>
List: netbsd-bugs
Date: 10/16/2006 10:00:05
The following reply was made to PR kern/21474; it has been noted by GNATS.

From: Peter Postma <peter@pointless.nl>
To: gnats-bugs@NetBSD.org
Cc: martin@NetBSD.org, tron@NetBSD.org
Subject: Re: kern/21474
Date: Mon, 16 Oct 2006 11:58:22 +0200

 --fUYQa+Pmc3FrFX/N
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 When the kernel crashes, the MTU on the interface is 0. ALTQ uses the MTU
 to calculate some things and then divides by zero in rmc_wrr_set_weights():
 
 >                        ifd->M_[i] = ifd->alloc_[i] /
 >                               (ifd->num_[i] * ifd->maxpkt_);
 
 ifd->maxpkt_ is set to the interface its MTU in rmc_init().
 
 I think that the fix would be to disallow a MTU < 1. The attached patch
 does that.
 
 -- 
 Peter Postma
 
 --fUYQa+Pmc3FrFX/N
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="altq.diff"
 
 Index: altq_rmclass.c
 ===================================================================
 RCS file: /cvsroot/src/sys/altq/altq_rmclass.c,v
 retrieving revision 1.15
 diff -u -r1.15 altq_rmclass.c
 --- altq_rmclass.c	12 Oct 2006 19:59:08 -0000	1.15
 +++ altq_rmclass.c	16 Oct 2006 09:52:55 -0000
 @@ -642,7 +642,7 @@
  
  
  /*
 - * void
 + * int
   * rmc_init(...) - Initialize the resource management data structures
   *	associated with the output portion of interface 'ifp'.  'ifd' is
   *	where the structures will be built (for backwards compatibility, the
 @@ -654,23 +654,29 @@
   *	is the maximum number of packets that the resource management
   *	code will allow to be queued 'downstream' (this is typically 1).
   *
 - *	Returns:	NONE
 + *	Returns:	0 on success
   */
  
 -void
 +int
  rmc_init(struct ifaltq *ifq, struct rm_ifdat *ifd, u_int nsecPerByte,
      void (*restart)(struct ifaltq *), int maxq, int maxqueued, u_int maxidle,
      int minidle, u_int offtime, int flags)
  {
 -	int		i, mtu;
 +	int i, mtu;
  
  	/*
  	 * Initialize the CBQ tracing/debug facility.
  	 */
  	CBQTRACEINIT();
  
 -	(void)memset((char *)ifd, 0, sizeof (*ifd));
  	mtu = ifq->altq_ifp->if_mtu;
 +	if (mtu < 1) {
 +		printf("altq: %s: invalid MTU (interface not initialized?)\n",
 +		    ifq->altq_ifp->if_xname);
 +		return (EINVAL);
 +	}
 +
 +	(void)memset((char *)ifd, 0, sizeof (*ifd));
  	ifd->ifq_ = ifq;
  	ifd->restart = restart;
  	ifd->maxqueued_ = maxqueued;
 @@ -718,9 +724,11 @@
  				       maxidle, minidle, offtime,
  				       0, 0)) == NULL) {
  		printf("rmc_init: root class not allocated\n");
 -		return ;
 +		return (ENOMEM);
  	}
  	ifd->root_->depth_ = 0;
 +
 +	return (0);
  }
  
  /*
 Index: altq_rmclass.h
 ===================================================================
 RCS file: /cvsroot/src/sys/altq/altq_rmclass.h,v
 retrieving revision 1.7
 diff -u -r1.7 altq_rmclass.h
 --- altq_rmclass.h	12 Oct 2006 19:59:08 -0000	1.7
 +++ altq_rmclass.h	16 Oct 2006 09:52:55 -0000
 @@ -248,7 +248,7 @@
  extern void	rmc_delete_class(struct rm_ifdat *, struct rm_class *);
  extern int 	rmc_modclass(struct rm_class *, u_int, int,
  			     u_int, int, u_int, int);
 -extern void	rmc_init(struct ifaltq *, struct rm_ifdat *, u_int,
 +extern int	rmc_init(struct ifaltq *, struct rm_ifdat *, u_int,
  			 void (*)(struct ifaltq *),
  			 int, int, u_int, int, u_int, int);
  extern int	rmc_queue_packet(struct rm_class *, mbuf_t *);
 Index: altq_cbq.c
 ===================================================================
 RCS file: /cvsroot/src/sys/altq/altq_cbq.c,v
 retrieving revision 1.20
 diff -u -r1.20 altq_cbq.c
 --- altq_cbq.c	13 Oct 2006 09:57:28 -0000	1.20
 +++ altq_cbq.c	16 Oct 2006 09:52:56 -0000
 @@ -314,7 +314,7 @@
  	cbq_state_t	*cbqp;
  	struct rm_class	*cl;
  	struct cbq_opts	*opts;
 -	int		i;
 +	int		i, error;
  
  	if ((cbqp = a->altq_disc) == NULL)
  		return (EINVAL);
 @@ -389,10 +389,12 @@
  	 * interface.
  	 */
  	if ((opts->flags & CBQCLF_CLASSMASK) == CBQCLF_ROOTCLASS) {
 -		rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp, opts->ns_per_byte,
 -		    cbqrestart, a->qlimit, RM_MAXQUEUED,
 +		error = rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp,
 +		    opts->ns_per_byte, cbqrestart, a->qlimit, RM_MAXQUEUED,
  		    opts->maxidle, opts->minidle, opts->offtime,
  		    opts->flags);
 +		if (error != 0)
 +			return (error);
  		cl = cbqp->ifnp.root_;
  	} else {
  		cl = rmc_newclass(a->priority,
 @@ -704,7 +706,7 @@
  	struct rm_class	*cl;
  	cbq_class_spec_t *spec = &acp->cbq_class;
  	u_int32_t	chandle;
 -	int		i;
 +	int		i, error;
  
  	/*
  	 * allocate class handle
 @@ -721,10 +723,12 @@
  	 * interface.
  	 */
  	if ((spec->flags & CBQCLF_CLASSMASK) == CBQCLF_ROOTCLASS) {
 -		rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp, spec->nano_sec_per_byte,
 -			 cbqrestart, spec->maxq, RM_MAXQUEUED,
 -			 spec->maxidle, spec->minidle, spec->offtime,
 -			 spec->flags);
 +		error = rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp,
 +		    spec->nano_sec_per_byte, cbqrestart, spec->maxq,
 +		    RM_MAXQUEUED, spec->maxidle, spec->minidle, spec->offtime,
 +		    spec->flags);
 +		if (error)
 +			return (error);
  		cl = cbqp->ifnp.root_;
  	} else {
  		cl = rmc_newclass(spec->priority,
 
 --fUYQa+Pmc3FrFX/N--