On Mon Mar 28 11, Maksim Yevmenkin wrote:
> On Mon, Mar 28, 2011 at 2:55 PM, Alexander Best <[email protected]> wrote:
> > On Mon Mar 28 11, Maksim Yevmenkin wrote:
> >> On Mon, Mar 28, 2011 at 2:37 PM, Alexander Best <[email protected]> 
> >> 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 
> >> >> > <[email protected]> wrote:
> >> >> > > On Mon Mar 28 11, Maksim Yevmenkin wrote:
> >> >> > >> On Mon, Mar 28, 2011 at 7:04 AM, Iain Hibbert 
> >> >> > >> <[email protected]> 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
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 [email protected]
+.An Maksim Yevmenkin Aq [email protected]
 .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 <bluetooth.h>
 #include <err.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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");
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bluetooth
To unsubscribe, send any mail to "[email protected]"

Reply via email to