* [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)
@ 2023-03-23 9:40 Dan Carpenter
2023-03-23 13:06 ` Tom Talpey
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2023-03-23 9:40 UTC (permalink / raw)
To: oe-kbuild, Shyam Prasad N
Cc: lkp, oe-kbuild-all, linux-cifs, samba-technical, Steve French,
Paulo Alcantara (SUSE)
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@intel.com/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@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202303230210.ufS9gVzi-lkp@intel.com/
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).
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 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 4+ messages in thread
* 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)
2023-03-23 9:40 [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) Dan Carpenter
@ 2023-03-23 13:06 ` Tom Talpey
2023-03-23 14:02 ` Shyam Prasad N
2023-03-23 16:40 ` Steve French
0 siblings, 2 replies; 4+ messages in thread
From: Tom Talpey @ 2023-03-23 13:06 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, Shyam Prasad N
Cc: lkp, oe-kbuild-all, linux-cifs, samba-technical, Steve French,
Paulo Alcantara (SUSE)
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@intel.com/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@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202303230210.ufS9gVzi-lkp@intel.com/
>
> 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 }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* 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)
2023-03-23 13:06 ` Tom Talpey
@ 2023-03-23 14:02 ` Shyam Prasad N
2023-03-23 16:40 ` Steve French
1 sibling, 0 replies; 4+ messages in thread
From: Shyam Prasad N @ 2023-03-23 14:02 UTC (permalink / raw)
To: Tom Talpey
Cc: Dan Carpenter, oe-kbuild, Shyam Prasad N, lkp, oe-kbuild-all,
linux-cifs, samba-technical, Steve French, Paulo Alcantara (SUSE)
On Thu, Mar 23, 2023 at 7:10 PM Tom Talpey <tom@talpey.com> 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@intel.com/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@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Link: https://lore.kernel.org/r/202303230210.ufS9gVzi-lkp@intel.com/
> >
> > 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* 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)
2023-03-23 13:06 ` Tom Talpey
2023-03-23 14:02 ` Shyam Prasad N
@ 2023-03-23 16:40 ` Steve French
1 sibling, 0 replies; 4+ messages in thread
From: Steve French @ 2023-03-23 16:40 UTC (permalink / raw)
To: Tom Talpey
Cc: Dan Carpenter, oe-kbuild, Shyam Prasad N, Paulo Alcantara (SUSE),
linux-cifs, lkp, samba-technical, oe-kbuild-all, Steve French
I have removed that patch from for-next (so still has the other 8
multichannel fixes and debug improvements). Should be adding an
important deferred close fix later this afternoon and the "umount -f"
fix as well
On Thu, Mar 23, 2023 at 8:07 AM Tom Talpey via samba-technical
<samba-technical@lists.samba.org> 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@intel.com/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@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Link: https://lore.kernel.org/r/202303230210.ufS9gVzi-lkp@intel.com/
> >
> > 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,
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-23 16:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23 9:40 [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) Dan Carpenter
2023-03-23 13:06 ` Tom Talpey
2023-03-23 14:02 ` Shyam Prasad N
2023-03-23 16:40 ` Steve French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).