Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 31 Jul 2008 15:21:36 +0200
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Patrick Lamaizi?re <patfbsd@davenulle.org>
Cc:        current@freebsd.org
Subject:   Re: Recent Padlock changes break ssh
Message-ID:  <20080731132136.GC4088@garage.freebsd.pl>
In-Reply-To: <20080731123246.365d0b1f@baby-jane-lamaiziere-net.local>
References:  <E1KLA49-0000W2-I1@clue.co.za> <20080722081449.GA3241@garage.freebsd.pl> <20080731123246.365d0b1f@baby-jane-lamaiziere-net.local>

next in thread | previous in thread | raw e-mail | index | archive | help

--HCdXmnRlPgeNBad2
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jul 31, 2008 at 12:32:46PM +0200, Patrick Lamaizi?re wrote:
> Le Tue, 22 Jul 2008 10:14:49 +0200,
> Pawel Jakub Dawidek <pjd@FreeBSD.org> a =E9crit :
>=20
> Hello,
>=20
> > Could you try this patch? Those are the only changes that could
> > eventually change the behaviour.
> >=20
> > 	http://people.freebsd.org/~pjd/patches/padlock.c.patch
> >=20
>=20
> I think that one problem is that the session id (ses->ses_id) is not
> updated when a free session is reused. The session id is set to zero by
> bzero() in padlock_freesession(). So we can have several active
> sessions with the same ses->ses_id =3D=3D 0 if the sessions are reused.

Great catch! What do you think about using old sessid? I think it's ok
to do so and a bit safer, because session ID is only 32bit long so we
may get collision once we start from 0 again.

> padlock_freession()
>  	padlock_hash_free(ses);
>  	bzero(ses, sizeof(*ses));
>  	ses->ses_used =3D 0;
> 	TAILQ_INSERT_HEAD(&sc->sc_sessions, ses, ses_next);
>=20
> and in padlock_newsession()
> 	/*
> 	 * Free sessions goes first, so if first session is used, we
> need to
> 	 * allocate one.
> 	 */
> 	ses =3D TAILQ_FIRST(&sc->sc_sessions);
> 	if (ses =3D=3D NULL || ses->ses_used)
> 		ses =3D NULL;
> 	else {
> 		TAILQ_REMOVE(&sc->sc_sessions, ses, ses_next);
> 		ses->ses_used =3D 1;
> +		ses->ses_id =3D sc->sc_sid++;
> 		TAILQ_INSERT_TAIL(&sc->sc_sessions, ses, ses_next);

I'd replace 'sc->sc_sid++' with 'sid'.

--=20
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd@FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!

--HCdXmnRlPgeNBad2
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)

iD8DBQFIkbxgForvXbEpPzQRAlc8AJkBKkgcFFuUD7BaitAgIlpS/tnvYgCgmVZ9
M2hckhua5657EKQ0fAayfPM=
=7HAT
-----END PGP SIGNATURE-----

--HCdXmnRlPgeNBad2--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080731132136.GC4088>