On 3/23/2023 5:40 AM, Dan Carpenter wrote:
tree: git://git.samba.org/sfrench/cifs-2.6.git for-next
head: 96114df697dfaef2ce29c14089a83e4a5777e915
commit: 010c4e0a894e6a3dee3176aa2f654fce632d0346 [3/8] cifs: fix sockaddr comparison in iface_cmp
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230323/202303230210.ufS9gVzi-lkp@xxxxxxxxx/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Reported-by: Dan Carpenter <error27@xxxxxxxxx>
| Link: https://lore.kernel.org/r/202303230210.ufS9gVzi-lkp@xxxxxxxxx/
New smatch warnings:
fs/cifs/connect.c:1303 cifs_ipaddr_cmp() error: memcmp() '&saddr4->sin_addr.s_addr' too small (4 vs 16)
fs/cifs/connect.c:1318 cifs_ipaddr_cmp() error: memcmp() '&saddr6->sin6_addr' too small (16 vs 28)
Old smatch warnings:
fs/cifs/connect.c:1303 cifs_ipaddr_cmp() error: memcmp() '&vaddr4->sin_addr.s_addr' too small (4 vs 16)
fs/cifs/connect.c:1318 cifs_ipaddr_cmp() error: memcmp() '&vaddr6->sin6_addr' too small (16 vs 28)
fs/cifs/connect.c:2937 generic_ip_connect() error: we previously assumed 'socket' could be null (see line 2925)
vim +1303 fs/cifs/connect.c
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1279 int
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1280 cifs_ipaddr_cmp(struct sockaddr *srcaddr, struct sockaddr *rhs)
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1281 {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1282 struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1283 struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1284 struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1285 struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1286
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1287 switch (srcaddr->sa_family) {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1288 case AF_UNSPEC:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1289 switch (rhs->sa_family) {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1290 case AF_UNSPEC:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1291 return 0;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1292 case AF_INET:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1293 case AF_INET6:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1294 return 1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1295 default:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1296 return -1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1297 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1298 case AF_INET: {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1299 switch (rhs->sa_family) {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1300 case AF_UNSPEC:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1301 return -1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1302 case AF_INET:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 @1303 return memcmp(&saddr4->sin_addr.s_addr,
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1304 &vaddr4->sin_addr.s_addr,
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1305 sizeof(struct sockaddr_in));
saddr4 and vaddr4 are type sockaddr_in. But sin_addr.s_addr is an
offset into the struct. This looks like a read overflow. I would think
it should be sizeof(struct in_addr).
Oh, definitely. It's more than a read overflow, it's an incorrect
comparison which will lead to creating new and unnecessary channels.
Two bugs here.
Tom.
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1306 case AF_INET6:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1307 return 1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1308 default:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1309 return -1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1310 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1311 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1312 case AF_INET6: {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1313 switch (rhs->sa_family) {
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1314 case AF_UNSPEC:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1315 case AF_INET:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1316 return -1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1317 case AF_INET6:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 @1318 return memcmp(&saddr6->sin6_addr,
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1319 &vaddr6->sin6_addr,
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1320 sizeof(struct sockaddr_in6));
Same.
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1321 default:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1322 return -1;
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1323 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1324 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1325 default:
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1326 return -1; /* don't expect to be here */
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1327 }
010c4e0a894e6a3 Shyam Prasad N 2022-12-27 1328 }
