Skip to content

Commit

Permalink
eeprom24xx: avoid access to caller's data after validity ends
Browse files Browse the repository at this point in the history
The at24 EEPROM decoder's previous implementation happened to access
caller's data even after the .decode() method invocation ended, and
their content has changed or the data was not valid any longer.

Get deep copies for those details which broke the test suite. Prepare
"generous" deep copies for other data which currently doesn't trigger
exceptions, but might be waiting for an accident to happen. Careful
inspection of the complex implementation and relaxing the current greed
of this commit remains for future commits. Comment heavily for awareness.

It is assumed that the 'databyte' name is misleading. And that much of
this upper layer decoder's complexity would be obsoleted by the lower
layer decoder's providing more useful packets (bytes and their ACK state,
read/write phases of transfers, complete transfers up to STOP, etc).
This commit does not address those readability or layering concerns.
  • Loading branch information
gsigh committed Jul 18, 2023
1 parent bb6f9c5 commit 42fb0f3
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions decoders/eeprom24xx/pd.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
## along with this program; if not, see <http://www.gnu.org/licenses/>.
##

import copy
import sigrokdecode as srd
from .lists import *

Expand Down Expand Up @@ -416,16 +417,25 @@ def handle_get_stop_after_last_byte(self):
self.reset_variables()

def decode(self, ss, es, data):
self.cmd, self.databyte = data
cmd, _ = data

# Collect the 'BITS' packet, then return. The next packet is
# guaranteed to belong to these bits we just stored.
if self.cmd == 'BITS':
self.bits = self.databyte
if cmd == 'BITS':
_, databits = data
self.bits = copy.deepcopy(databits)
return

# Store the start/end samples of this I²C packet.
# Store the start/end samples of this I²C packet. Deep copy
# caller's data, assuming that implementation details of the
# above complex methods can access the data after returning
# from the .decode() invocation, with the data having become
# invalid by that time of access. This conservative approach
# can get weakened after close inspection of those methods.
self.ss, self.es = ss, es
_, databyte = data
databyte = copy.deepcopy(databyte)
self.cmd, self.databyte = cmd, databyte

# State machine.
s = 'handle_%s' % self.state.lower().replace(' ', '_')
Expand Down

0 comments on commit 42fb0f3

Please sign in to comment.