Adjusts the access violation checks at BPF ABIv0 and ABIv1 deserializ…

…ation.
solana-labs · Jun 10, 2022 · cb0f075 · cb0f075
1 parent 1ff6253
commit cb0f075
Showing 1 changed file with 36 additions and 19 deletions.
diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs
@@ -169,18 +169,25 @@ pub fn deserialize_parameters_unaligned(
             start += size_of::<u8>(); // is_signer
             start += size_of::<u8>(); // is_writable
             start += size_of::<Pubkey>(); // key
-            let _ = borrowed_account.set_lamports(LittleEndian::read_u64(
+            let lamports = LittleEndian::read_u64(
                 buffer
                     .get(start..)
                     .ok_or(InstructionError::InvalidArgument)?,
-            ));
+            );
+            if borrowed_account.get_lamports() != lamports {
+                borrowed_account.set_lamports(lamports)?;
+            }
             start += size_of::<u64>() // lamports
                 + size_of::<u64>(); // data length
-            let _ = borrowed_account.set_data(
-                buffer
-                    .get(start..start + pre_len)
-                    .ok_or(InstructionError::InvalidArgument)?,
-            );
+            let data = buffer
+                .get(start..start + pre_len)
+                .ok_or(InstructionError::InvalidArgument)?;
+            // The redundant `is_data_mutable` helps to avoid the expensive data comparison if we can
+            if borrowed_account.is_data_mutable(data.len()).is_ok()
+                || borrowed_account.get_data() != data
+            {
+                borrowed_account.set_data(data)?;
+            }
             start += pre_len // data
                 + size_of::<Pubkey>() // owner
                 + size_of::<u8>() // executable
@@ -310,17 +317,18 @@ pub fn deserialize_parameters_aligned(
                 + size_of::<u8>() // executable
                 + 4 // padding to 128-bit aligned
                 + size_of::<Pubkey>(); // key
-            let _ = borrowed_account.set_owner(
-                buffer
-                    .get(start..start + size_of::<Pubkey>())
-                    .ok_or(InstructionError::InvalidArgument)?,
-            );
+            let owner = buffer
+                .get(start..start + size_of::<Pubkey>())
+                .ok_or(InstructionError::InvalidArgument)?;
             start += size_of::<Pubkey>(); // owner
-            let _ = borrowed_account.set_lamports(LittleEndian::read_u64(
+            let lamports = LittleEndian::read_u64(
                 buffer
                     .get(start..)
                     .ok_or(InstructionError::InvalidArgument)?,
-            ));
+            );
+            if borrowed_account.get_lamports() != lamports {
+                borrowed_account.set_lamports(lamports)?;
+            }
             start += size_of::<u64>(); // lamports
             let post_len = LittleEndian::read_u64(
                 buffer
@@ -344,14 +352,23 @@ pub fn deserialize_parameters_aligned(
                 }
                 data_end
             };
-            let _ = borrowed_account.set_data(
-                buffer
-                    .get(start..data_end)
-                    .ok_or(InstructionError::InvalidArgument)?,
-            );
+            let data = buffer
+                .get(start..data_end)
+                .ok_or(InstructionError::InvalidArgument)?;
+            // The redundant `is_data_mutable` helps to avoid the expensive data comparison if we can
+            if borrowed_account.is_data_mutable(data.len()).is_ok()
+                || borrowed_account.get_data() != data
+            {
+                borrowed_account.set_data(data)?;
+            }
             start += *pre_len + MAX_PERMITTED_DATA_INCREASE; // data
             start += (start as *const u8).align_offset(BPF_ALIGN_OF_U128);
             start += size_of::<u64>(); // rent_epoch
+
+            // Change the owner at the end so that we are allowed to change the lamports and data before
+            if borrowed_account.get_owner().to_bytes() != owner {
+                borrowed_account.set_owner(owner)?;
+            }
         }
     }
     Ok(())