From: Gerhard Sittig <redacted>
Date: Fri, 14 Jul 2023 06:12:13 +0000 (+0200)
Subject: eeprom24xx: avoid access to caller's data after validity ends
X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=42fb0f33ff23c1633324bde8b3cf400493237d97;p=libsigrokdecode.git

eeprom24xx: avoid access to caller's data after validity ends

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.
---

diff --git a/decoders/eeprom24xx/pd.py b/decoders/eeprom24xx/pd.py
index 549ee2d..7491f58 100644
--- a/decoders/eeprom24xx/pd.py
+++ b/decoders/eeprom24xx/pd.py
@@ -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 *
 
@@ -416,16 +417,25 @@ class Decoder(srd.Decoder):
             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(' ', '_')