Hi Gordon,
----- Original Message -----
From: "Gordon Tyler" <[EMAIL PROTECTED]>
To: "WinPcap Users List" <[EMAIL PROTECTED]>
Sent: Wednesday, July 17, 2002 9:21 PM
Subject: Re: [WinPcap-users] Access Violation
> Further information from our investigating developer:
>
> I was finally able to download the latest sources of winpcap (see attached
> file Read.c).
> Everything about the DLL remains true, but the driver code seems to have
> been fixed in a number of places.
> So the scenario from the previous message will not fail just as it was
> described because in the new source the order of 'free space' checks has
> been corrected (Read.c, Packet_tap(), lines 351--368):
> New code:
> if((Ttail < Thead) && (Ttail+maxbufspace+1 >= Thead))
> {
> //the buffer is full: the incoming packet is lost
> Open->Dropped++;
> return NDIS_STATUS_NOT_ACCEPTED;
> }
> if(Ttail+maxbufspace >= Open->BufSize){
> if(Thead<=maxbufspace)
> {
> //the buffer is full: the packet is lost
> Open->Dropped++;
> return NDIS_STATUS_NOT_ACCEPTED;
> }
> else{
> Ttail=0;
> }
> }
> ---------------------------------------------
> Old Code:
> maxbufspace=fres+sizeof(struct bpf_hdr);
> if(Ttail+maxbufspace>=Open->BufSize){
> if(Thead<=maxbufspace)
> {
> //the buffer is full: the packet is lost
> Open->Dropped++;
> return NDIS_STATUS_NOT_ACCEPTED;
> }
> else{
> Ttail=0;
> }
> }
>
> if((Ttail<Thead)&&(Ttail+maxbufspace>=Thead))
> {
> //the buffer is full: the incoming packet is lost
> Open->Dropped++;
> return NDIS_STATUS_NOT_ACCEPTED;
> }
>
> Nonetheless, it is still possible for the driver to get into an error
prone
> state.
> Let's assume the buffer occupancy is:
>
> X X X X X X X X X * * * * * * * * * * X X X *
> ^ ^ ^
> Bt Bh Bl
> where Bt = Btail, Bh = Bhead, Bl = BLastByte, X - occupied bytes, * - free
> bytes.
> If PacketRead and Packet_tap are called at the same time they retrieve
those
> pointers into each one's Txxx local variables.
> Then PacketRead sends the wrapped portion of the buffer to the DLL while
> Packet_tap appends a new packet to the buffer.
> Assuming that PacketRead completes before Packet_tap, then Packet_tap
moves
> Btail forward and sets BLastByte to the old value because in its view head
>
> tail (lines 447--455):
> * * * * * * * * * Y Y Y Y Y Y Y Y Y * * * * *
> ^ ^ ^
> Bh Bt Bl
> where Y - new bytes.
No. Since head > tail, the assignment doesn't takes place, so BLastByte
keeps the value set by PacketRead.
Notice that, with a single execution of of PacketRead, the situation cannot
be the one that you describe, because a single PacketRead would copy only
the portion between Bh and Bl, setting Open->BLastByte = Open->Btail.
More important, notice your analysis is based on the assumption that
PacketRead and Packet_tap have the same priority, without any
synchronization. This is not completely true because the tap, being invoked
by the underlying kernel layers, runs at dispatch (DPC) level, while
PacketRead runs at passive level, the one of normal application scheduling.
This means that Packet_tap can never be interrupted by PacketRead, and that
if Packet_tap is running PacketRead is never called.
> Then another call to PacketRead sends the rest of data to the DLL but does
> not change BLastByte (lines 189--196),
> and a call to Packet_tap puts a new packet at the beginning of the buffer
> (it does not fit at the end) but since again tail < head it does not
change
> BLastByte (line 450). The resulting buffer state is:
> X X X X X X * * * * * * * * * * * * * * * * *
> ^ ^ ^
> Bt Bh Bl
> Now, if PacketRead is called it will read 'free' space, and chances are
> garbage will be there.
> As a summary I'd like to say that operations with BLastByte are not so
> clear -- Packet_tap sets it after adding a packet to the buffer, while it
> seems it should set BLastByte when the buffer wraps.
Rather than this, I think that it would be cleaner if only one of the two
functions (Packet_tap and PacketRead) could modify BLastByte. From this
point of view, I think that the assignments
Open->BLastByte = Open->Btail
in PacketRead (line 203 and 227) could be removed without bringing
inconsistencies.
Loris
> I hope the assumptions I made are correct.
>
>
