On Thu, Mar 23, 2023 at 7:10 PM Tom Talpey <tom@xxxxxxxxxx> wrote: > > 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 } > > Thanks for catching this Dan. I will fix this and send an updated patch. -- Regards, Shyam
