From owner-freebsd-bluetooth@FreeBSD.ORG Mon Mar 28 22:52:08 2011 Return-Path: Delivered-To: freebsd-bluetooth@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id 2AD0E1065678; Mon, 28 Mar 2011 22:52:08 +0000 (UTC) Date: Mon, 28 Mar 2011 22:52:08 +0000 From: Alexander Best To: Maksim Yevmenkin Message-ID: <20110328225208.GA51932@freebsd.org> References: <20110328101804.GA39095@freebsd.org> <20110328195952.GA26987@freebsd.org> <20110328203413.GB26987@freebsd.org> <20110328213746.GA42088@freebsd.org> <20110328215503.GA43845@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="UlVJffcvxoiEqYs2" Content-Disposition: inline In-Reply-To: Cc: freebsd-bluetooth@freebsd.org Subject: Re: l2ping(8) and -f switch X-BeenThere: freebsd-bluetooth@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Using Bluetooth in FreeBSD environments List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Mar 2011 22:52:08 -0000 --UlVJffcvxoiEqYs2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon Mar 28 11, Maksim Yevmenkin wrote: > On Mon, Mar 28, 2011 at 2:55 PM, Alexander Best wrote: > > On Mon Mar 28 11, Maksim Yevmenkin wrote: > >> On Mon, Mar 28, 2011 at 2:37 PM, Alexander Best wrote: > >> > On Mon Mar 28 11, Alexander Best wrote: > >> >> On Mon Mar 28 11, Maksim Yevmenkin wrote: > >> >> > On Mon, Mar 28, 2011 at 12:59 PM, Alexander Best wrote: > >> >> > > On Mon Mar 28 11, Maksim Yevmenkin wrote: > >> >> > >> On Mon, Mar 28, 2011 at 7:04 AM, Iain Hibbert wrote: > >> >> > >> > On Mon, 28 Mar 2011, Alexander Best wrote: > >> >> > >> > > >> >> > >> >> On Mon Mar 28 11, Iain Hibbert wrote: > >> >> > >> >> > On Mon, 28 Mar 2011, Alexander Best wrote: > >> >> > >> >> > > >> >> > >> >> > > thus i believe making the -f switch only accessable to super-users (in > >> >> > >> >> > > accordance with ping(8)/ping6(8)) would increase security. > >> >> > >> >> > > >> >> > >> >> > what stops the user from recompiling l2ping without this restriction? > >> >> > >> >> > >> >> > >> >> nothing. but what stops him from recompiling ping(8) or ping6(8) without the > >> >> > >> >> restriction? still it's there. > >> >> > >> > > >> >> > >> > AFAIK you need superuser privileges to even send ICMP_ECHO packets, thats > >> >> > >> > why ping is traditionally a suid program and making a new binary won't > >> >> > >> > help normal users.. I'm guessing that l2ping doesn't have the same > >> >> > >> > restrictions? > >> >> > >> > >> >> > >> Guys, > >> >> > >> > >> >> > >> first of all thanks for the patch. > >> >> > >> > >> >> > >> i think one really needs to understand what "flood" really means in > >> >> > >> l2ping(8). "flood" ping(8) basically floods the link with icmp echo > >> >> > >> requests without waiting for remote system to reply. yes, this is > >> >> > >> potentially dangerous and thus its reasonable to require super-user > >> >> > >> privileges. "flood" l2ping(8) is NOT the same. all l2ping(8) does is > >> >> > >> "flood" mode > >> >> > >> > >> >> > >> 1) sends l2cap echo request > >> >> > >> 2) waits for l2cap echo response (or timeout) > >> >> > >> 3) repeats > >> >> > >> > >> >> > >> in other words, there is no delay between each l2cap echo > >> >> > >> request-response transaction. its not really "flood". i'm not sure if > >> >> > >> it really worth to go all the way to restricting this. however, if > >> >> > >> people think that it should be restricted, i will not object. > >> >> > > > >> >> > > how about removing the term "flood" from the l2ping(2) man page, if the -f > >> >> > > semantics can't actually be called that way? > >> >> > > >> >> > that would be fine. l2ping(8) -h calls it > >> >> > > >> >> > -f No delay (sort of flood) > >> >> > > >> >> > and l2ping(8) man page calls it > >> >> > > >> >> > -f ``Flood'' ping, i.e., no delay between packets. > >> >> > > >> >> > it would be nice to make those consistent :) i'm not sure what the > >> >> > best name would be though. > >> >> > >> >> another possibility would be to allow l2ping -i 0 and say that the -f flag is > >> >> an alias for that. > >> > >> the existing code provides exactly this behavior. perhaps just a man > >> page and usage() change? > > > > hmmm...no actually. l2ping -i 0 is not a valid parameter, since -i has to be > > greater than 0. so it's not possible to simply say "-f is an alias for -i 0", > > because that implies that -i 0 should work (which it doesn't). > > well, don't call it an "alias" then :) call it "effectively -i 0", "no > delay" or something like that :) > > >> > the following patch will implement this behavior. > >> > >> if we are going to go this route then why not just get rid of the > >> "flood" variable all together? just set wait to 0 (zero) if -f was > >> specified. also, we should probably use strtol(3) instead of atoi(3). > > > > i've thought of that. however that would mean l2ping -f -i 3 would set the > > delay to 3 seconds and usually an -f switch implies "force". so i think the > > current behavior of -f having a higher priority than any -i X option should be > > kept. > > i think that this is not worthy of long discussion :) i agree that > word 'flood' is not appropriate and/or confusing. all the patches > provided were fine, imo. please make a decision and commit the best > (in your opinion) fix. sorry, but i don't own a src commit bit. however i think the following patch should be fine. i've also eliminated a few var = NULL, since they're all being initialised properly and unconditionally at some point. also the defenitions didn't apply to style(9). plus i've converted all the atoi() calls to strtol() calls. cheers. alex > > thank you > > max -- a13x --UlVJffcvxoiEqYs2 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="l2ping.diff" diff --git a/usr.sbin/bluetooth/l2ping/l2ping.8 b/usr.sbin/bluetooth/l2ping/l2ping.8 index 477f4ec..703b0bd 100644 --- a/usr.sbin/bluetooth/l2ping/l2ping.8 +++ b/usr.sbin/bluetooth/l2ping/l2ping.8 @@ -25,7 +25,7 @@ .\" $Id: l2ping.8,v 1.3 2003/05/21 01:00:19 max Exp $ .\" $FreeBSD$ .\" -.Dd June 14, 2002 +.Dd March 29, 2011 .Dt L2PING 8 .Os .Sh NAME @@ -36,7 +36,7 @@ .Op Fl fhn .Fl a Ar remote .Op Fl c Ar count -.Op Fl i Ar delay +.Op Fl i Ar wait .Op Fl S Ar source .Op Fl s Ar size .Sh DESCRIPTION @@ -63,8 +63,7 @@ If this option is not specified, .Nm will operate until interrupted. .It Fl f -.Dq Flood -ping, i.e., no delay between packets. +Don't wait between sending each packet. .It Fl h Display usage message and exit. .It Fl i Ar wait @@ -109,7 +108,7 @@ Some implementations may not like large sizes and may hang or even crash. .Xr ng_l2cap 4 , .Xr l2control 8 .Sh AUTHORS -.An Maksim Yevmenkin Aq m_evmenkin@yahoo.com +.An Maksim Yevmenkin Aq emax@FreeBSD.org .Sh BUGS Could collect more statistic. Could check for duplicated, corrupted and lost packets. diff --git a/usr.sbin/bluetooth/l2ping/l2ping.c b/usr.sbin/bluetooth/l2ping/l2ping.c index d7e1b1e..4baa354 100644 --- a/usr.sbin/bluetooth/l2ping/l2ping.c +++ b/usr.sbin/bluetooth/l2ping/l2ping.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -60,11 +61,11 @@ int main(int argc, char *argv[]) { bdaddr_t src, dst; - struct hostent *he = NULL; - uint8_t *echo_data = NULL; + struct hostent *he; + uint8_t *echo_data; struct sockaddr_l2cap sa; int32_t n, s, count, wait, flood, echo_size, numeric; - char *rname = NULL; + char *endp, *rname; /* Set defaults */ memcpy(&src, NG_HCI_BDADDR_ANY, sizeof(src)); @@ -100,8 +101,8 @@ main(int argc, char *argv[]) break; case 'c': - count = atoi(optarg); - if (count <= 0) + count = strtol(optarg, &endp, 10); + if (count <= 0 || *endp != '\0') usage(); break; @@ -110,8 +111,8 @@ main(int argc, char *argv[]) break; case 'i': - wait = atoi(optarg); - if (wait <= 0) + wait = strtol(optarg, &endp, 10); + if (wait <= 0 || *endp != '\0') usage(); break; @@ -129,9 +130,10 @@ main(int argc, char *argv[]) break; case 's': - echo_size = atoi(optarg); - if (echo_size < sizeof(int32_t) || - echo_size > NG_L2CAP_MAX_ECHO_SIZE) + echo_size = strtol(optarg, &endp, 10); + if (echo_size < sizeof(int32_t) || + echo_size > NG_L2CAP_MAX_ECHO_SIZE || + *endp != '\0') usage(); break; @@ -272,12 +274,12 @@ tv2msec(struct timeval const *tvp) static void usage(void) { - fprintf(stderr, "Usage: l2ping -a bd_addr " \ - "[-S bd_addr -c count -i wait -n -s size -h]\n"); + fprintf(stderr, "Usage: l2ping [-fhn] -a remote " \ + "[-c count] [-i wait] [-S source] [-s size]\n"); fprintf(stderr, "Where:\n"); fprintf(stderr, " -a remote Specify remote device to ping\n"); fprintf(stderr, " -c count Number of packets to send\n"); - fprintf(stderr, " -f No delay (sort of flood)\n"); + fprintf(stderr, " -f No delay between packets\n"); fprintf(stderr, " -h Display this message\n"); fprintf(stderr, " -i wait Delay between packets (sec)\n"); fprintf(stderr, " -n Numeric output only\n"); --UlVJffcvxoiEqYs2--