[Freeciv-Dev] Re: (PR#13383) Memory error in hunter code

["Marko Lindqvist" <cazfi74@xxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13383 >

Jason Short wrote:
> 
> Marko Lindqvist wrote:
>>
>>  From valgrind:
>>
>> ==18897== Invalid read of size 4
>> ==18897==    at 0x8100249: ai_hunter_try_launch (aihunt.c:261)
>> ==18897==    by 0x8100CD5: ai_hunter_manage (aihunt.c:495)
>> ==18897==    by 0x810946F: ai_manage_military (aiunit.c:1960)
>> ==18897==    by 0x810A566: ai_manage_unit (aiunit.c:2149)
>> ==18897==    by 0x810B1A1: ai_manage_units (aiunit.c:2248)
>> ==18897==    by 0x80FF72D: ai_do_first_activities (aihand.c:426)
>> ==18897==    by 0x8051039: main_loop (srv_main.c:453)
>> ==18897==    by 0x8051AC2: srv_main (srv_main.c:1962)
>> ==18897==    by 0x804A99A: main (civserver.c:242)
>> ==18897==  Address 0x25E3D7AC is 4 bytes inside a block of size 192 free'd
>> ==18897==    at 0x2598579D: free (vg_replace_malloc.c:152)
>> ==18897==    by 0x805BDC6: server_remove_unit (unittools.c:1605)
>> ==18897==    by 0x805F044: wipe_unit_spec_safe (unittools.c:1666)
>> ==18897==    by 0x805FAA0: kill_unit (unittools.c:1831)
>> ==18897==    by 0x809B246: handle_unit_attack_request (unithand.c:899)
>> ==18897==    by 0x809AC25: handle_unit_move_request (unithand.c:1120)
>> ==18897==    by 0x8104EFE: ai_unit_attack (aitools.c:921)
>> ==18897==    by 0x81050B4: ai_unit_execute_path (aitools.c:162)
>> ==18897==    by 0x8100CBB: ai_hunter_manage (aihunt.c:489)
>> ==18897==    by 0x810946F: ai_manage_military (aiunit.c:1960)
>> ==18897==    by 0x810A566: ai_manage_unit (aiunit.c:2149)
>> ==18897==    by 0x810B1A1: ai_manage_units (aiunit.c:2248)
> 
> Can you reproduce this?
> 
> Clearly the unit is dying inside ai_unit_execute_path called from
> aihunt.c:489.  A little later in aihunt.c:495 the unit is accessed.
> However it looks like the aihunt code correctly checks the return value
> of ai_unit_execute_path.  So you'd think the bug is inside
> ai_unit_execut_path.  But here there is a correct call to
> find_unit_by_id.  So I don't see how this can happen.

  Ah, illegal read is actually about target, not about hunter. Hunter 
may kill target inside ai_unit_execute_path() and later we try to access 
target inside ai_hunter_try_launch().
  Untested patch attached.


  - ML

diff -Nurd -X.diff_ignore freeciv/ai/aihunt.c freeciv/ai/aihunt.c
--- freeciv/ai/aihunt.c 2006-07-20 21:38:16.062500000 +0300
+++ freeciv/ai/aihunt.c 2006-07-21 00:05:06.125000000 +0300
@@ -505,6 +505,13 @@
         return 0;
       }
 
+      if (target != find_unit_by_id(sanity_target)) {
+        UNIT_LOG(LOGLEVEL_HUNT, punit, "mission accomplished");
+        ai_unit_new_role(punit, AIUNIT_NONE, NULL);
+        pf_destroy_map(map);
+        return -1; /* try again */
+      }
+
       /* Check if we can nuke it now */
       ai_hunter_try_launch(pplayer, punit, target);
       if (target != find_unit_by_id(sanity_target)) {