Fix sloppy CAN_(EFF|RTR)_FLAG handling in can_filter.can_mask .

Due to a wrong safety check in af_can.c it was not possible to filter
for SFF frames with a specific CAN identifier without getting the
same selected CAN identifier from a received EFF frame also.

This fix has a minimum impact on the CAN filter API as the 'sloppy'
handling is still a correct (and possibly wanted?) use-case.

Please update the can-utils (especially candump) whose filter definition
on the commandline made assumptions to correct the user input that are
probably unwanted now.

Thanks to Kurt van Dijck for pointing at this issue!

Signed-Off-by: Oliver Hartkopp <oliver@hartkopp.net>

diff --git a/candump.c b/candump.c
index 940f81de5ec1193577231398ecf71e405674d223..f2762f9e0bc4cae3cc8f392b815e54d665ca1176 100644 (file)
--- a/candump.c
+++ b/candump.c
@@ -130,8 +130,9 @@ void print_usage(char *prg)
        fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n", prg);
        fprintf(stderr, "%s -l any,0~0,#FFFFFFFF    (log only error frames but no(!) data frames)\n", prg);
        fprintf(stderr, "%s -l any,0:0,#FFFFFFFF    (log error frames and also all data frames)\n", prg);
-       fprintf(stderr, "%s vcan2,92345678:9FFFFFFF (match only for extended CAN ID 12345678)\n", prg);
-       fprintf(stderr, "%s vcan2,12345678:1FFFFFFF (analog to above due to 8 digit value length)\n", prg);
+       fprintf(stderr, "%s vcan2,92345678:DFFFFFFF (match only for extended CAN ID 12345678)\n", prg);
+       fprintf(stderr, "%s vcan2,123:7FF (matches CAN ID 123 - including EFF and RTR frames)\n", prg);
+       fprintf(stderr, "%s vcan2,123:C00007FF (matches CAN ID 123 - only SFF and non-RTR frames)\n", prg);
        fprintf(stderr, "\n");
 }
 
@@ -189,21 +190,6 @@ int idx2dindex(int ifidx, int socket) {
        return i;
 }
 
-canid_t checkeff(char *ptr, char *nptr)
-{
-       int len;
-
-       if (nptr)
-               len = nptr - ptr;
-       else
-               len = strlen(ptr);
-
-       if (len == 17 && (ptr[8] == ':' || ptr[8] == '~'))
-               return CAN_EFF_FLAG;
-       else
-               return 0;
-}
-
 int main(int argc, char **argv)
 {
        fd_set rdfs;
@@ -219,7 +205,6 @@ int main(int argc, char **argv)
        int opt, ret;
        int currmax, numfilter;
        char *ptr, *nptr;
-       canid_t eff;
        struct sockaddr_can addr;
        struct can_filter rfilter[MAXFILTER];
        can_err_mask_t err_mask;
@@ -411,9 +396,6 @@ int main(int argc, char **argv)
                                           &rfilter[numfilter].can_id, 
                                           (long unsigned int *)
                                           &rfilter[numfilter].can_mask) == 2) {
-                                       eff = checkeff(ptr, nptr);
-                                       rfilter[numfilter].can_id |= eff;
-                                       rfilter[numfilter].can_mask |= eff;
                                        rfilter[numfilter].can_mask &= ~CAN_ERR_FLAG;
                                        numfilter++;
                                } else if (sscanf(ptr, "%lx~%lx",
@@ -422,9 +404,6 @@ int main(int argc, char **argv)
                                                  (long unsigned int *)
                                                  &rfilter[numfilter].can_mask) == 2) {
                                        rfilter[numfilter].can_id |= CAN_INV_FILTER;
-                                       eff = checkeff(ptr, nptr);
-                                       rfilter[numfilter].can_id |= eff;
-                                       rfilter[numfilter].can_mask |= eff;
                                        rfilter[numfilter].can_mask &= ~CAN_ERR_FLAG;
                                        numfilter++;
                                } else if (sscanf(ptr, "#%lx",

diff --git a/isotpdump.c b/isotpdump.c
index 22b81414e302f25c92b52e86cfcf254908f60393..c5c4de5afb703df96fe90f3e1c8489d171c203f7 100644 (file)
--- a/isotpdump.c
+++ b/isotpdump.c
@@ -162,17 +162,22 @@ int main(int argc, char **argv)
                return 1;
        }
 
-       rfilter[0].can_id   = src;
-       if (src & CAN_EFF_FLAG)
-               rfilter[0].can_mask = CAN_EFF_MASK | CAN_EFF_FLAG;
-       else
-               rfilter[0].can_mask = CAN_SFF_MASK;
-
-       rfilter[1].can_id   = dst;
-       if (dst & CAN_EFF_FLAG)
-               rfilter[1].can_mask = CAN_EFF_MASK | CAN_EFF_FLAG;
-       else
-               rfilter[1].can_mask = CAN_SFF_MASK;
+
+       if (src & CAN_EFF_FLAG) {
+               rfilter[0].can_id   = src & (CAN_EFF_MASK | CAN_EFF_FLAG);
+               rfilter[0].can_mask = (CAN_EFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+       } else {
+               rfilter[0].can_id   = src & CAN_SFF_MASK;
+               rfilter[0].can_mask = (CAN_SFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+       }
+
+       if (dst & CAN_EFF_FLAG) {
+               rfilter[1].can_id   = dst & (CAN_EFF_MASK | CAN_EFF_FLAG);
+               rfilter[1].can_mask = (CAN_EFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+       } else {
+               rfilter[1].can_id   = dst & CAN_SFF_MASK;
+               rfilter[1].can_mask = (CAN_SFF_MASK|CAN_EFF_FLAG|CAN_RTR_FLAG);
+       }
 
        setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER, &rfilter, sizeof(rfilter));