Request to postpone WE-21

Request to postpone WE-21

[Jean Tourrilhes]

	Hi John,

	Based on the feedback, I formally request you to back out all
of WE-21 from 2.6.19. Rationale : it's probably too early. You can
keep it for a later date if you wish.
	Regards,

	Jean

Re: Request to postpone WE-21

[John W. Linville]

On Thu, Oct 05, 2006 at 09:31:13AM -0700, Jean Tourrilhes wrote:

> 	Based on the feedback, I formally request you to back out all
> of WE-21 from 2.6.19. Rationale : it's probably too early. You can
> keep it for a later date if you wish.

Jean,

What about a patch like the one below?  It tries to detect WE-20
ESSID/NICKN accesses and adjust them to WE-21 style.  What am
I missing?

I haven't had a chance to test it yet -- just hacked it
up...YMMV... :-)

John

diff --git a/net/core/wireless.c b/net/core/wireless.c
index ffff0da..ba5fe77 100644
--- a/net/core/wireless.c
+++ b/net/core/wireless.c
@@ -748,11 +748,39 @@ #endif	/* WE_SET_EVENT */
 		int	extra_size;
 		int	user_length = 0;
 		int	err;
+		int	essid_compat = 0;
 
 		/* Calculate space needed by arguments. Always allocate
 		 * for max space. Easier, and won't last long... */
 		extra_size = descr->max_tokens * descr->token_size;
 
+		/* Check need for ESSID compatibility for WE < 21 */
+		switch (cmd) {
+		case SIOCSIWESSID:
+		case SIOCGIWESSID:
+		case SIOCSIWNICKN:
+		case SIOCGIWNICKN:
+			if (iwr->u.data.length == descr->max_tokens + 1)
+				essid_compat = 1;
+			else if (IW_IS_SET(cmd)) {
+				char essid[IW_ESSID_MAX_SIZE + 1];
+
+				err = copy_from_user(essid, iwr->u.data.pointer,
+						     iwr->u.data.length *
+						     descr->token_size);
+				if (err)
+					return -EFAULT;
+
+				if (essid[iwr->u.data.length] == '\0')
+					essid_compat = 1;
+			}
+			break;
+		default:
+			break;
+		}
+
+		iwr->u.data.length -= essid_compat;
+
 		/* Check what user space is giving us */
 		if(IW_IS_SET(cmd)) {
 			/* Check NULL pointer */
@@ -795,7 +823,8 @@ #ifdef WE_IOCTL_DEBUG
 #endif	/* WE_IOCTL_DEBUG */
 
 		/* Create the kernel buffer */
-		extra = kmalloc(extra_size, GFP_KERNEL);
+		/*    kzalloc ensures NULL-termination for essid_compat */
+		extra = kzalloc(extra_size, GFP_KERNEL);
 		if (extra == NULL) {
 			return -ENOMEM;
 		}
@@ -819,6 +848,8 @@ #endif	/* WE_IOCTL_DEBUG */
 		/* Call the handler */
 		ret = handler(dev, &info, &(iwr->u), extra);
 
+		iwr->u.data.length += essid_compat;
+
 		/* If we have something to return to the user */
 		if (!ret && IW_IS_GET(cmd)) {
 			/* Check if there is enough buffer up there */

-- 
John W. Linville
linville@tuxdriver.com

Re: Request to postpone WE-21

[Jean Tourrilhes]

On Thu, Oct 05, 2006 at 04:49:54PM -0400, John W. Linville wrote:
> On Thu, Oct 05, 2006 at 09:31:13AM -0700, Jean Tourrilhes wrote:
> 
> > 	Based on the feedback, I formally request you to back out all
> > of WE-21 from 2.6.19. Rationale : it's probably too early. You can
> > keep it for a later date if you wish.
> 
> Jean,

	Let me say I truly apreciate your effort to bring progress to
the discussion.

> What about a patch like the one below?  It tries to detect WE-20
> ESSID/NICKN accesses and adjust them to WE-21 style.  What am
> I missing?

	The idea is clever.
	The GET is no longer an issue. WE had half the driver doing
the GET "new style" since january, so in a sense the API change has
already happened, and I've already dealt with the bug reports. So, I
think we could drop the "GET" part.
	As you may have noticed, detecting the API for the GET is
easy. On the other hand, detecting it for the SET is not clear cut. As
Jouni was pointing out, '\0' is a valid ESSID character, and in the
long term we want to allow it, even if it's in the last position.
	I'm also wondering if this additional complexity could not
bring additional trouble, but I'm not currently clear on that. I
usually prefer things to be a bit more explicit.

> I haven't had a chance to test it yet -- just hacked it
> up...YMMV... :-)

	And I thing there is a couple of way we could refine the
implementation, if ever we decide to go that way.
	For example, the correction could happen after real
copy_from_user(), as the uncorrected iwr->u.data.length is always the
number of char to pass between kernel and userspace. I think this
would simplify drastically the code.
	I'll try to check that.

> John

	Thanks again...

	Jean

Re: Request to postpone WE-21

[Jouni Malinen]

On Thu, Oct 05, 2006 at 04:49:54PM -0400, John W. Linville wrote:

> What about a patch like the one below?  It tries to detect WE-20
> ESSID/NICKN accesses and adjust them to WE-21 style.  What am
> I missing?

> diff --git a/net/core/wireless.c b/net/core/wireless.c

> +			else if (IW_IS_SET(cmd)) {
> +				char essid[IW_ESSID_MAX_SIZE + 1];
> +
> +				err = copy_from_user(essid, iwr->u.data.pointer,
> +						     iwr->u.data.length *
> +						     descr->token_size);

> +				if (essid[iwr->u.data.length] == '\0')
> +					essid_compat = 1;

This looks somewhat confusing.. WE-20 (and older) included '\0' in both
the data value and length (well, at least in most drivers and user space
tools, if I remember correctly), i.e., essid[iwr->u.data.length] would
be pointing one byte after the '\0' termination.. And since '\0' is
valid character in SSID (it is just an arbitrary array of octets) it can
also be the last octet of the SSID and WE-21 style case could have
essid[iwr->u.data.length - 1] == '\0'..

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: Request to postpone WE-21

[Jean Tourrilhes]

On Thu, Oct 05, 2006 at 02:57:51PM -0700, Jouni Malinen wrote:
> On Thu, Oct 05, 2006 at 04:49:54PM -0400, John W. Linville wrote:
> 
> > What about a patch like the one below?  It tries to detect WE-20
> > ESSID/NICKN accesses and adjust them to WE-21 style.  What am
> > I missing?
> 
> > diff --git a/net/core/wireless.c b/net/core/wireless.c
> 
> > +			else if (IW_IS_SET(cmd)) {
> > +				char essid[IW_ESSID_MAX_SIZE + 1];
> > +
> > +				err = copy_from_user(essid, iwr->u.data.pointer,
> > +						     iwr->u.data.length *
> > +						     descr->token_size);
> 
> > +				if (essid[iwr->u.data.length] == '\0')
> > +					essid_compat = 1;
> 
> This looks somewhat confusing.. WE-20 (and older) included '\0' in both
> the data value and length (well, at least in most drivers and user space
> tools, if I remember correctly), i.e., essid[iwr->u.data.length] would
> be pointing one byte after the '\0' termination..

	Obviously. John's code was only a proof of concept, tested
patch is coming in another e-mail.

> And since '\0' is
> valid character in SSID (it is just an arbitrary array of octets) it can
> also be the last octet of the SSID and WE-21 style case could have
> essid[iwr->u.data.length - 1] == '\0'..

	I think we will have to suffer a bit. After the big mess
created by the whole story, I think it might not be a bad idea to get
90% of the way there, and add the remaining 10% of a later date. At
the rate userspace is progressing, it's only a matter of weeks.

> Jouni Malinen

	Have fun...

	Jean

Re: Request to postpone WE-21

[Jeff Garzik]

Jouni Malinen wrote:
> This looks somewhat confusing.. WE-20 (and older) included '\0' in both
> the data value and length (well, at least in most drivers and user space
> tools, if I remember correctly), i.e., essid[iwr->u.data.length] would
> be pointing one byte after the '\0' termination.. And since '\0' is
> valid character in SSID (it is just an arbitrary array of octets) it can
> also be the last octet of the SSID and WE-21 style case could have
> essid[iwr->u.data.length - 1] == '\0'..


Remember, the salient point is ensuring that WE<=20 continues to work as 
expecting, without any modification.  If that means a compromise in 
supported SSID values, so be it.  Just like older versions of stat(2) 
syscall, you are stuck with the old interface, warts included.

But if we can support both styles... great!  I'm all for it.  Just 
noting priorities.  Warts and limitations are inevitable with older 
interfaces.

	Jeff

Re: Request to postpone WE-21

[Jean Tourrilhes]

On Thu, Oct 05, 2006 at 04:49:54PM -0400, John W. Linville wrote:
> On Thu, Oct 05, 2006 at 09:31:13AM -0700, Jean Tourrilhes wrote:
> 
> > 	Based on the feedback, I formally request you to back out all
> > of WE-21 from 2.6.19. Rationale : it's probably too early. You can
> > keep it for a later date if you wish.
> 
> Jean,
> 
> What about a patch like the one below?  It tries to detect WE-20
> ESSID/NICKN accesses and adjust them to WE-21 style.  What am
> I missing?
> 
> I haven't had a chance to test it yet -- just hacked it
> up...YMMV... :-)
> 
> John

	This is a tested and simplified version of your patch. I added
an explicit "warning" message, so that users have visibility on what's
happening.
	I had a bit of fun on the testing phase, as most of my antique
cards were just discarding the extra '\0'. The Broadcom did show the
issue, and that this patch was fixing it properly.
	I let you decide the best course of action.

	Have fun...

	Jean

Signed-off-by: Jean Tourrilhes <jt@hpl.hp.com>

--------------------------------------------------

diff -u -p linux/net/core/wireless.j1.c linux/net/core/wireless.c
--- linux/net/core/wireless.j1.c	2006-10-05 14:24:16.000000000 -0700
+++ linux/net/core/wireless.c	2006-10-05 14:53:23.000000000 -0700
@@ -821,6 +821,26 @@ static int ioctl_standard_call(struct ne
 			       dev->name,
 			       iwr->u.data.length * descr->token_size);
 #endif	/* WE_IOCTL_DEBUG */
+
+			/* Some users have old userspace compatible
+			 * with WE-20. Those add an extra '\0' at the
+			 * end of the ESSID. Just get rids of it.
+			 * Note : in the long term we will need to get
+			 * rid of this code to allow '\0' as a valid
+			 * last char of the ESSID.
+			 * Jean II */
+			if((cmd == SIOCSIWESSID) ||
+			   (cmd == SIOCSIWNICKN)) {
+				if(extra[iwr->u.data.length - 1] == '\0') {
+					static int printed_message;
+
+					if (!printed_message++)
+						printk(KERN_DEBUG "%s (WE) : NUL terminated ESSID detected, please update Wireless Tools !\n",
+						       dev->name);
+
+					iwr->u.data.length--;
+				}
+			}
 		}
 
 		/* Call the handler */

Re: Request to postpone WE-21

[Jouni Malinen]

On Thu, Oct 05, 2006 at 03:12:46PM -0700, Jean Tourrilhes wrote:
> +			if((cmd == SIOCSIWESSID) ||
> +			   (cmd == SIOCSIWNICKN)) {
> +				if(extra[iwr->u.data.length - 1] == '\0') {

Can't iwr->u.data.length be zero here (with WE-21)? Maybe better add
'iwr->u.data.length > 0 &&'..

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: Request to postpone WE-21

[Jean Tourrilhes]

On Thu, Oct 05, 2006 at 03:15:50PM -0700, Jouni Malinen wrote:
> On Thu, Oct 05, 2006 at 03:12:46PM -0700, Jean Tourrilhes wrote:
> > +			if((cmd == SIOCSIWESSID) ||
> > +			   (cmd == SIOCSIWNICKN)) {
> > +				if(extra[iwr->u.data.length - 1] == '\0') {
> 
> Can't iwr->u.data.length be zero here (with WE-21)? Maybe better add
> 'iwr->u.data.length > 0 &&'..

	It's already implied. I did not put the code in the same place
a John.
	It looks like :
----------------------------------------------------------
		/* If it is a SET, get all the extra data in here */
		if(IW_IS_SET(cmd) && (iwr->u.data.length != 0)) {
			err = copy_from_user(extra, iwr->u.data.pointer,
					     iwr->u.data.length *
					     descr->token_size);

			if((cmd == SIOCSIWESSID) ||
			   (cmd == SIOCSIWNICKN)) {
				if(extra[iwr->u.data.length - 1] == '\0') {
					iwr->u.data.length--;
				}
			}
		}
----------------------------------------------------------

> Jouni Malinen

	Thanks for the quick review !

	Jean

Re: Request to postpone WE-21

[John W. Linville]

On Thu, Oct 05, 2006 at 03:12:46PM -0700, Jean Tourrilhes wrote:
> On Thu, Oct 05, 2006 at 04:49:54PM -0400, John W. Linville wrote:
> > On Thu, Oct 05, 2006 at 09:31:13AM -0700, Jean Tourrilhes wrote:
> > 
> > > 	Based on the feedback, I formally request you to back out all
> > > of WE-21 from 2.6.19. Rationale : it's probably too early. You can
> > > keep it for a later date if you wish.
> > 
> > Jean,
> > 
> > What about a patch like the one below?  It tries to detect WE-20
> > ESSID/NICKN accesses and adjust them to WE-21 style.  What am
> > I missing?
> > 
> > I haven't had a chance to test it yet -- just hacked it
> > up...YMMV... :-)
> > 
> > John
> 
> 	This is a tested and simplified version of your patch. I added
> an explicit "warning" message, so that users have visibility on what's
> happening.
> 	I had a bit of fun on the testing phase, as most of my antique
> cards were just discarding the extra '\0'. The Broadcom did show the
> issue, and that this patch was fixing it properly.
> 	I let you decide the best course of action.

I think this patch still has two problems.  One is that the length
modification does not happen until after the "Check what user space
is giving us" clause.  So, max length requests will fail.  (Did you
check SIOCGIWESSID w/ WE-20 tools?  Or am I missing something?
SIOCGIWESSID and SIOCGIWNICN do not have IW_DESCR_FLAG_NOMAX set.)

Since I think we have to account for the gets as well as the sets,
then we have to restore the length value in the ifreq to be compatible
with userland, just in case they reuse the data structure.

I think the main issues with my patch were an off-by-one in the length
for checking for \0, and a failure to account for a length of zero.
I have attempted to correct those in my patch below.  What do you
think?

John

P.S.  Again, this is an untested patch...

diff --git a/net/core/wireless.c b/net/core/wireless.c
index ffff0da..cb1b872 100644
--- a/net/core/wireless.c
+++ b/net/core/wireless.c
@@ -748,11 +748,39 @@ #endif	/* WE_SET_EVENT */
 		int	extra_size;
 		int	user_length = 0;
 		int	err;
+		int	essid_compat = 0;
 
 		/* Calculate space needed by arguments. Always allocate
 		 * for max space. Easier, and won't last long... */
 		extra_size = descr->max_tokens * descr->token_size;
 
+		/* Check need for ESSID compatibility for WE < 21 */
+		switch (cmd) {
+		case SIOCSIWESSID:
+		case SIOCGIWESSID:
+		case SIOCSIWNICKN:
+		case SIOCGIWNICKN:
+			if (iwr->u.data.length == descr->max_tokens + 1)
+				essid_compat = 1;
+			else if (IW_IS_SET(cmd) && (iwr->u.data.length != 0)) {
+				char essid[IW_ESSID_MAX_SIZE + 1];
+
+				err = copy_from_user(essid, iwr->u.data.pointer,
+						     iwr->u.data.length *
+						     descr->token_size);
+				if (err)
+					return -EFAULT;
+
+				if (essid[iwr->u.data.length - 1] == '\0')
+					essid_compat = 1;
+			}
+			break;
+		default:
+			break;
+		}
+
+		iwr->u.data.length -= essid_compat;
+
 		/* Check what user space is giving us */
 		if(IW_IS_SET(cmd)) {
 			/* Check NULL pointer */
@@ -795,7 +823,8 @@ #ifdef WE_IOCTL_DEBUG
 #endif	/* WE_IOCTL_DEBUG */
 
 		/* Create the kernel buffer */
-		extra = kmalloc(extra_size, GFP_KERNEL);
+		/*    kzalloc ensures NULL-termination for essid_compat */
+		extra = kzalloc(extra_size, GFP_KERNEL);
 		if (extra == NULL) {
 			return -ENOMEM;
 		}
@@ -819,6 +848,8 @@ #endif	/* WE_IOCTL_DEBUG */
 		/* Call the handler */
 		ret = handler(dev, &info, &(iwr->u), extra);
 
+		iwr->u.data.length += essid_compat;
+
 		/* If we have something to return to the user */
 		if (!ret && IW_IS_GET(cmd)) {
 			/* Check if there is enough buffer up there */
-- 
John W. Linville
linville@tuxdriver.com

Re: Request to postpone WE-21

[Jean Tourrilhes]

On Tue, Oct 10, 2006 at 03:40:04PM -0400, John W. Linville wrote:
> 
> I think this patch still has two problems.  One is that the length
> modification does not happen until after the "Check what user space
> is giving us" clause.  So, max length requests will fail.  (Did you
> check SIOCGIWESSID w/ WE-20 tools?  Or am I missing something?
> SIOCGIWESSID and SIOCGIWNICN do not have IW_DESCR_FLAG_NOMAX set.)

	Yes, I can see that SET with 32 char would get rejected. I did
not check 32 char with WE-20 tools. I don't know how widely 32 char
ESSID are used in the real world.

	Over the week end, I tought about another potential gotcha,
however I don't think it would happen in practice. A tool could send a
non NUL terminated ESSID with WE-20, just adding a dummy char at the
end of the ESSID. That would work with most drivers.

> Since I think we have to account for the gets as well as the sets,
> then we have to restore the length value in the ifreq to be compatible
> with userland, just in case they reuse the data structure.

	The GET API change *already* happened last January. It's
already a reality, and WE-21 did not really change anything about
that. This means that it's already included in "2.6.16", which is the
"somewhat more stable" kernel, and widely diffused.
	There was userspace issues with that. Those have been fixed a
long time ago. I don't see the point of carrying code for those cases.

> I think the main issues with my patch were an off-by-one in the length
> for checking for \0, and a failure to account for a length of zero.
> I have attempted to correct those in my patch below.  What do you
> think?
> 
> John
> 
> P.S.  Again, this is an untested patch...

	Compile Wireless Tools 27 with a static iwlib (top of
Makefile) to do your testing, so that you can run without having to
install it. And use a driver that care about the ending NUL, such as
the Broadcom or IPW2x00.

	Have fun...

	Jean

Please pull 'we21-fix' branch of wireless-2.6.git

[John W. Linville]

Jeff,

Here is my ugly patch to fix userland ABI compatibility for WE-21.
It tries to detect WE <= 20 by the request length or the inclusion of
'\0' in the length for the ESSID and NICKN ioctls.  If it finds that,
it temporarily adjusts the length value and puts it back before
reporting back to userland.

It is possible that there are more elegant solutions that work, but
I'm not totally convinced that they truly preserve the ABI as desired.
Please see the discussion earlier in this thread if you are interested.

I'm aware that checking for '\0' is problematic, since that technically
is a valid SSID character even at the end of the SSID.  I'm afraid
we will just have to live with that limitation.

I have the single patch on its own "clean" branch from Linus's tree of
a few days ago.  That way you can pull just this fix without concern
about picking-up the other fixes which I posted a few days ago,
in case you haven't reviewed them yet.

Obviously, this is intended for 2.6.19.

Thanks,

John
---

The following changes since commit 51018b0a3160d253283173c2f54f16746cee5852:
  Ulrich Drepper:
        make UML compile (FC6/x86-64)

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/wireless-2.6.git we21-fix

John W. Linville:
      wireless: WE-20 compatibility for ESSID and NICKN ioctls

 net/core/wireless.c |   33 ++++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/net/core/wireless.c b/net/core/wireless.c
index ffff0da..cb1b872 100644
--- a/net/core/wireless.c
+++ b/net/core/wireless.c
@@ -748,11 +748,39 @@ #endif	/* WE_SET_EVENT */
 		int	extra_size;
 		int	user_length = 0;
 		int	err;
+		int	essid_compat = 0;
 
 		/* Calculate space needed by arguments. Always allocate
 		 * for max space. Easier, and won't last long... */
 		extra_size = descr->max_tokens * descr->token_size;
 
+		/* Check need for ESSID compatibility for WE < 21 */
+		switch (cmd) {
+		case SIOCSIWESSID:
+		case SIOCGIWESSID:
+		case SIOCSIWNICKN:
+		case SIOCGIWNICKN:
+			if (iwr->u.data.length == descr->max_tokens + 1)
+				essid_compat = 1;
+			else if (IW_IS_SET(cmd) && (iwr->u.data.length != 0)) {
+				char essid[IW_ESSID_MAX_SIZE + 1];
+
+				err = copy_from_user(essid, iwr->u.data.pointer,
+						     iwr->u.data.length *
+						     descr->token_size);
+				if (err)
+					return -EFAULT;
+
+				if (essid[iwr->u.data.length - 1] == '\0')
+					essid_compat = 1;
+			}
+			break;
+		default:
+			break;
+		}
+
+		iwr->u.data.length -= essid_compat;
+
 		/* Check what user space is giving us */
 		if(IW_IS_SET(cmd)) {
 			/* Check NULL pointer */
@@ -795,7 +823,8 @@ #ifdef WE_IOCTL_DEBUG
 #endif	/* WE_IOCTL_DEBUG */
 
 		/* Create the kernel buffer */
-		extra = kmalloc(extra_size, GFP_KERNEL);
+		/*    kzalloc ensures NULL-termination for essid_compat */
+		extra = kzalloc(extra_size, GFP_KERNEL);
 		if (extra == NULL) {
 			return -ENOMEM;
 		}
@@ -819,6 +848,8 @@ #endif	/* WE_IOCTL_DEBUG */
 		/* Call the handler */
 		ret = handler(dev, &info, &(iwr->u), extra);
 
+		iwr->u.data.length += essid_compat;
+
 		/* If we have something to return to the user */
 		if (!ret && IW_IS_GET(cmd)) {
 			/* Check if there is enough buffer up there */
-- 
John W. Linville
linville@tuxdriver.com

Re: Please pull 'we21-fix' branch of wireless-2.6.git

[Jean Tourrilhes]

On Thu, Oct 19, 2006 at 05:56:53PM -0400, John W. Linville wrote:
> Jeff,
> 
> Here is my ugly patch to fix userland ABI compatibility for WE-21.
> It tries to detect WE <= 20 by the request length or the inclusion of
> '\0' in the length for the ESSID and NICKN ioctls.  If it finds that,
> it temporarily adjusts the length value and puts it back before
> reporting back to userland.
> 
> It is possible that there are more elegant solutions that work, but
> I'm not totally convinced that they truly preserve the ABI as desired.
> Please see the discussion earlier in this thread if you are interested.

	I agree that this is a good compromise.
	Thanks a lot for your work, John !

	Jean

Re: Please pull 'we21-fix' branch of wireless-2.6.git

[Jeff Garzik]

John W. Linville wrote:
> Jeff,
> 
> Here is my ugly patch to fix userland ABI compatibility for WE-21.
> It tries to detect WE <= 20 by the request length or the inclusion of
> '\0' in the length for the ESSID and NICKN ioctls.  If it finds that,
> it temporarily adjusts the length value and puts it back before
> reporting back to userland.
> 
> It is possible that there are more elegant solutions that work, but
> I'm not totally convinced that they truly preserve the ABI as desired.
> Please see the discussion earlier in this thread if you are interested.
> 
> I'm aware that checking for '\0' is problematic, since that technically
> is a valid SSID character even at the end of the SSID.  I'm afraid
> we will just have to live with that limitation.
> 
> I have the single patch on its own "clean" branch from Linus's tree of
> a few days ago.  That way you can pull just this fix without concern
> about picking-up the other fixes which I posted a few days ago,
> in case you haven't reviewed them yet.
> 
> Obviously, this is intended for 2.6.19.
> 
> Thanks,
> 
> John
> ---
> 
> The following changes since commit 51018b0a3160d253283173c2f54f16746cee5852:
>   Ulrich Drepper:
>         make UML compile (FC6/x86-64)
> 
> are found in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/wireless-2.6.git we21-fix

pulled, after adding "git/linville/" to the URL...

	Jeff