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 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




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

  Powered by Linux