rlm_perl bug while adding VSA?

[magmike at mail.ru]

>>> magmike at mail.ru wrote:
>>>>   If you're running a 5 year-old version of the server, you really need
>>>> to upgrade.

>>> i use freeradiusd 2.1.12
>>> $ freeradius -v
>>> freeradius: FreeRADIUS Version 2.1.12, for host i486-pc-linux-gnu, built on Dec 16 2012 at 22:03:33

>>   <shrug>  It works for me with the latest release.

>>   Please try with 2.2.3.  If the bug is still there, I'll look at it.

>>   Alan DeKok.

> It's like bug in functions which use VALUE_PAIR.attribute  as signed int

> i.e.  paircopy2 from valuepair.c:359

> -------------------------------------------------------------
> VALUE_PAIR *paircopy2(VALUE_PAIR *vp, int attr)
> {
>     VALUE_PAIR  *first, *n, **last;

>     first = NULL;
>     last = &first;

>     while (vp) {
>         if (attr >= 0 && vp->attribute != attr) {
>             vp = vp->next;
>             continue;
>         }
> -------------------------------------------------------------

> Here when vendor id > 32K, attr argument is negative.
> so  
>   if (attr >= 0 && vp->attribute != attr) {
> will false 

>  attr >= 0 should be changed to attr == -1 for correct execution



Patch for 2.1.12:
----------------------------------------------------------------------------
--- src/lib/valuepair.c.orig    2014-01-17 17:54:53.105873692 +0600
+++ src/lib/valuepair.c 2014-01-17 17:57:06.313879873 +0600
@@ -358,24 +358,25 @@
  */
 VALUE_PAIR *paircopy2(VALUE_PAIR *vp, int attr)
 {
-       VALUE_PAIR      *first, *n, **last;
-
-       first = NULL;
-       last = &first;
+       VALUE_PAIR* result = NULL;
+       VALUE_PAIR* tail = NULL;
+       VALUE_PAIR* tmpcopy;

        while (vp) {
-               if (attr >= 0 && vp->attribute != attr) {
-                       vp = vp->next;
-                       continue;
+               if (attr == -1 || vp->attribute == attr) {
+                       tmpcopy = paircopyvp(vp);
+                       tmpcopy->next = NULL;
+                       if (tail == NULL) {
+                               result = tail = tmpcopy;
+                       } else {
+                               tail->next = tmpcopy;
+                               tail = tmpcopy;
+                       }
                }
-
-               n = paircopyvp(vp);
-               if (!n) return first;
-               *last = n;
-               last = &n->next;
                vp = vp->next;
        }
-       return first;
+
+       return result;
 }
----------------------------------------------------------------------------
It's look work good.

Bug investigated and patch issued by my co-worker Dmitry Davletbaev <ddomgn at gmail.com>

Michael