Skip to content

Commit 1fc3232

Browse files
bryamzxzgregkh
authored andcommitted
nfc: fdp: bound the device-reported read length and fix an skb leak
commit 7ad21dc upstream. fdp_nci_i2c_read() takes the next packet length from two device-supplied bytes and never validates it. The value is a u16 used as the i2c_master_recv() count into a 261-byte on-stack buffer: a malicious, counterfeit or malfunctioning controller (or an i2c bus interposer) can drive it far past the buffer for a stack out-of-bounds write that clobbers the canary and return address, or below the minimum frame size (directly, or by truncating the computed sum) so the header/LRC strip and the next length read run past a short receive. Reject a length outside [FDP_NCI_I2C_MIN_PAYLOAD, FDP_NCI_I2C_MAX_PAYLOAD], as a corrupted packet already is, and force resynchronization. The same loop allocates one data skb per iteration and assumes a length packet followed by a data packet; a device that sends two data packets in one call leaks the first skb when the second allocation overwrites it. Free a previously allocated skb before allocating the next. Fixes: a06347c ("NFC: Add Intel Fields Peak NFC solution driver") Cc: stable@vger.kernel.org Suggested-by: Simon Horman <horms@kernel.org> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me> Link: https://patch.msgid.link/20260616-b4-disp-b1f8ab4c-v2-1-2d1fe5955325@proton.me Signed-off-by: David Heidelberg <david@ixit.cz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent e886c63 commit 1fc3232

1 file changed

Lines changed: 27 additions & 0 deletions

File tree

drivers/nfc/fdp/i2c.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,36 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
166166
/* Packet that contains a length */
167167
if (tmp[0] == 0 && tmp[1] == 0) {
168168
phy->next_read_size = (tmp[2] << 8) + tmp[3] + 3;
169+
170+
/*
171+
* next_read_size is taken from the device and is used
172+
* as the i2c_master_recv() count for the next packet
173+
* and as the data skb size. A value above the receive
174+
* buffer overflows tmp[]; one below the minimum frame
175+
* size runs the header/LRC strip and the length-field
176+
* read past a short receive. Either way the packet is
177+
* corrupt: drop it and force resynchronization.
178+
*/
179+
if (phy->next_read_size < FDP_NCI_I2C_MIN_PAYLOAD ||
180+
phy->next_read_size > FDP_NCI_I2C_MAX_PAYLOAD) {
181+
dev_dbg(&client->dev, "%s: corrupted packet\n",
182+
__func__);
183+
phy->next_read_size = FDP_NCI_I2C_MIN_PAYLOAD;
184+
goto flush;
185+
}
169186
} else {
170187
phy->next_read_size = FDP_NCI_I2C_MIN_PAYLOAD;
171188

189+
/*
190+
* Only one data packet is delivered per call; if the
191+
* device sends another, do not overwrite and leak the
192+
* skb allocated for the previous one.
193+
*/
194+
if (*skb) {
195+
kfree_skb(*skb);
196+
*skb = NULL;
197+
}
198+
172199
*skb = alloc_skb(len, GFP_KERNEL);
173200
if (*skb == NULL) {
174201
r = -ENOMEM;

0 commit comments

Comments
 (0)