Merge #19270 #19271

19270: drivers/at24cxxx: implement _mtd_at24cxxx_read_page r=benpicco a=HendrikVE

### Contribution description

The function `read_page` was missing which lead to (from a user perspective) undefined behavior on the MTD layer.

### Testing procedure

Any application using MTD in conjunction with a board with an at24cxxx.


19271: core/xfa: disable asan on llvm r=benpicco a=Teufelchen1

### Contribution description
Hi! 🦎

When using llvm and address sanitation, the XFA trip the sanitizer.
This PR attempts to fix this by adding the `no_sanitize` attribute to the XFA macros. Sadly, this attribute is not known by gnu, a guard is hence needed. I'm open for alternatives as I dislike this solution but it is the best I could come up with.

### Testing procedure

Before this patch:

Go to `examples/gnrc_minimal` and run `TOOLCHAIN=llvm make all-asan` and then `make term`.
You should see an error similar to this:
```
==3374719==ERROR: AddressSanitizer: global-buffer-overflow on address 0x080774e0 at pc 0x0804af5e bp 0x0808eb88 sp 0x0808eb78
READ of size 4 at 0x080774e0 thread T0
    #0 0x804af5d in _auto_init_module /RIOT/sys/auto_init/auto_init.c:40
    #1 0x804af5d in auto_init /RIOT/sys/auto_init/auto_init.c:339
    #2 0x804b375 in main_trampoline /RIOT/core/lib/init.c:56
    #3 0xf76bc7b8 in makecontext (/lib32/libc.so.6+0x4a7b8)
...
``` 
After applying this PR, the example can be build and run with llvm or gcc, with or without asan.



Co-authored-by: Hendrik van Essen <hendrik.vanessen@ml-pa.com>
Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>

core/lib/include/compiler_hints.h

@@ -68,6 +68,20 @@ extern "C" {
 #endif
 #endif
 
+/**
+ * @def       NO_SANITIZE_ARRAY
+ * @brief     Tell the compiler that this array should be ignored during sanitizing.
+ * @details   In special cases, e.g. XFA, the address sanitizer might interfere
+ *            in a way that breaks the application. Use this macro to disable
+ *            address sanitizing for a given variable. Currently only utilised
+ *            by llvm.
+ */
+#if defined(__llvm__) || defined(__clang__)
+#define NO_SANITIZE_ARRAY __attribute__((no_sanitize("address")))
+#else
+#define NO_SANITIZE_ARRAY
+#endif
+
 /**
  * @def       UNREACHABLE()
  * @brief     Tell the compiler that this line of code cannot be reached.

core/lib/include/xfa.h

@@ -26,6 +26,7 @@
 #define XFA_H
 
 #include <inttypes.h>
+#include "compiler_hints.h"
 
 /*
  * Unfortunately, current gcc trips over accessing XFA's because of their
@@ -42,16 +43,18 @@ _Pragma("GCC diagnostic ignored \"-Warray-bounds\"")
  *
  * @internal
  */
-#define _XFA(name, prio) __attribute__((used, section(".xfa." #name "." #prio)))
+#define _XFA(name, prio) \
+    NO_SANITIZE_ARRAY \
+    __attribute__((used, section(".xfa." #name "." #prio)))
 
 /**
  * @brief helper macro for other XFA_* macros
  *
  * @internal
  */
-#define _XFA_CONST(name, \
+#define _XFA_CONST(name, prio) \
-                   prio) __attribute__((used, \
+    NO_SANITIZE_ARRAY \
-                                        section(".roxfa." #name "." #prio)))
+    __attribute__((used, section(".roxfa." #name "." #prio)))
 
 /**
  * @brief Define a read-only cross-file array

cpu/sam0_common/include/periph_cpu_common.h

@@ -56,6 +56,11 @@ extern "C" {
 #define PERIPH_I2C_NEED_WRITE_REGS
 /** @} */
 
+/**
+ * @brief   Maximum bytes per frame for I2C operations
+ */
+#define PERIPH_I2C_MAX_BYTES_PER_FRAME 256
+
 /**
  * @brief   Override GPIO type
  * @{

drivers/at24cxxx/at24cxxx.c

@@ -97,17 +97,42 @@ int _read(const at24cxxx_t *dev, uint32_t pos, void *data, size_t len)
         }
         xtimer_usleep(AT24CXXX_POLL_DELAY_US);
     }
+
     DEBUG("[at24cxxx] i2c_read_regs(): %d; polls: %d\n", check, polls);
+
     return check;
 }
 
+static int _read_max(const at24cxxx_t *dev, uint32_t pos, void *data, size_t len)
+{
+#ifdef PERIPH_I2C_MAX_BYTES_PER_FRAME
+    uint8_t *data_p = data;
+
+    while (len) {
+        size_t clen = MIN(len, PERIPH_I2C_MAX_BYTES_PER_FRAME);
+
+        if (_read(dev, pos, data_p, clen) == AT24CXXX_OK) {
+            len -= clen;
+            pos += clen;
+            data_p += clen;
+        }
+        else {
+            return -EIO;
+        }
+    }
+
+    return AT24CXXX_OK;
+#else
+    return _read(dev, pos, data, len);
+#endif
+}
+
 static
 int _write_page(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t len)
 {
     int check;
     uint8_t polls = DEV_MAX_POLLS;
     uint8_t dev_addr;
-    uint16_t _pos;
     uint8_t flags = 0;
 
     if (DEV_EEPROM_SIZE > 2048) {
@@ -115,17 +140,17 @@ int _write_page(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t le
            used for addressing */
         /* append page address bits to device address (if any) */
         dev_addr  = (DEV_I2C_ADDR | ((pos & 0xFF0000) >> 16));
-        _pos = (pos & 0xFFFF);
+        pos &= 0xFFFF;
         flags = I2C_REG16;
     }
     else {
         /* append page address bits to device address (if any) */
         dev_addr = (DEV_I2C_ADDR | ((pos & 0xFF00) >> 8));
-        _pos = pos & 0xFF;
+        pos &= 0xFF;
     }
 
     while (-ENXIO == (check = i2c_write_regs(DEV_I2C_BUS, dev_addr,
-                                             _pos, data, len, flags))) {
+                                             pos, data, len, flags))) {
         if (--polls == 0) {
             break;
         }
@@ -209,8 +234,7 @@ int at24cxxx_read_byte(const at24cxxx_t *dev, uint32_t pos, void *dest)
     return check;
 }
 
-int at24cxxx_read(const at24cxxx_t *dev, uint32_t pos, void *data,
+int at24cxxx_read(const at24cxxx_t *dev, uint32_t pos, void *data, size_t len)
-                      size_t len)
 {
     if (pos + len > DEV_EEPROM_SIZE) {
         return -ERANGE;
@@ -219,9 +243,10 @@ int at24cxxx_read(const at24cxxx_t *dev, uint32_t pos, void *data,
     int check = AT24CXXX_OK;
     if (len) {
         i2c_acquire(DEV_I2C_BUS);
-        check = _read(dev, pos, data, len);
+        check = _read_max(dev, pos, data, len);
         i2c_release(DEV_I2C_BUS);
     }
+
     return check;
 }
 

drivers/at24cxxx/mtd/mtd.c

@@ -45,16 +45,11 @@ static int _mtd_at24cxxx_init(mtd_dev_t *mtd)
     return 0;
 }
 
-static int _mtd_at24cxxx_read(mtd_dev_t *mtd, void *dest, uint32_t addr,
+static int _mtd_at24cxxx_read_page(mtd_dev_t *mtd, void *dest, uint32_t page,
-                              uint32_t size)
+                                   uint32_t offset, uint32_t size)
 {
-    return at24cxxx_read(DEV(mtd), addr, dest, size) == AT24CXXX_OK ? 0 : -EIO;
+    int rc = at24cxxx_read(DEV(mtd), page * mtd->page_size + offset, dest, size);
-}
+    return rc == AT24CXXX_OK ? (int)size : rc;
-
-static int _mtd_at24cxxx_write(mtd_dev_t *mtd, const void *src, uint32_t addr,
-                               uint32_t size)
-{
-    return at24cxxx_write(DEV(mtd), addr, src, size) == AT24CXXX_OK ? 0 : -EIO;
 }
 
 static int mtd_at24cxxx_write_page(mtd_dev_t *mtd, const void *src, uint32_t page,
@@ -77,8 +72,7 @@ static int _mtd_at24cxxx_power(mtd_dev_t *mtd, enum mtd_power_state power)
 
 const mtd_desc_t mtd_at24cxxx_driver = {
     .init = _mtd_at24cxxx_init,
-    .read = _mtd_at24cxxx_read,
+    .read_page = _mtd_at24cxxx_read_page,
-    .write = _mtd_at24cxxx_write,
     .write_page = mtd_at24cxxx_write_page,
     .erase = _mtd_at24cxxx_erase,
     .power = _mtd_at24cxxx_power,

drivers/include/at24cxxx.h

@@ -96,8 +96,7 @@ int at24cxxx_read_byte(const at24cxxx_t *dev, uint32_t pos, void *dest);
  * @return          -ERANGE if @p pos + @p len is out of bounds
  * @return          @see i2c_read_regs
  */
-int at24cxxx_read(const at24cxxx_t *dev, uint32_t pos, void *data,
+int at24cxxx_read(const at24cxxx_t *dev, uint32_t pos, void *data, size_t len);
-                  size_t len);
 
 /**
  * @brief   Write a byte at a given position @p pos