Re: [cifs:for-next 3/8] fs/cifs/connect.c:1303 cifs_ipaddr_cmp() error: memcmp() '&saddr4->sin_addr.s_addr' too small (4 vs 16)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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  }




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux