From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29075 invoked by alias); 1 May 2013 20:11:44 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 29056 invoked by uid 89); 1 May 2013 20:11:44 -0000 X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS,TW_FD autolearn=ham version=3.3.1 X-Spam-User: qpsmtpd, 2 recipients Received: from mail-ye0-f172.google.com (HELO mail-ye0-f172.google.com) (209.85.213.172) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 01 May 2013 20:11:43 +0000 Received: by mail-ye0-f172.google.com with SMTP id m15so345694yen.17 for ; Wed, 01 May 2013 13:11:42 -0700 (PDT) X-Received: by 10.236.103.201 with SMTP id f49mr2889414yhg.55.1367439101998; Wed, 01 May 2013 13:11:41 -0700 (PDT) Received: from dhcp-191-132.bos.redhat.com ([66.187.233.206]) by mx.google.com with ESMTPSA id j27sm7583116yhf.18.2013.05.01.13.11.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 01 May 2013 13:11:41 -0700 (PDT) Message-ID: <518176FC.5030906@gmail.com> Date: Wed, 01 May 2013 20:11:00 -0000 From: KOSAKI Motohiro User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130328 Thunderbird/17.0.5 MIME-Version: 1.0 To: Carlos O'Donell CC: libc-alpha , "libc-ports@sourceware.org" , kosaki.motohiro@gmail.com Subject: Re: [PATCH 2/5] __FD_ELT: Implement correct buffer overflow check References: <1365900451-19026-1-git-send-email-kosaki.motohiro@gmail.com> <1365900451-19026-3-git-send-email-kosaki.motohiro@gmail.com> <518080FD.1090402@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00019.txt.bz2 >> Does compiling ruby (or similar code) with this header >> result in calls to __fdelt_buffer_warn or __fdelt_buffer_chk? > > Unfortunately, No. __builtin_object_size() require compiler know the > buffer size. > In the other words, it doesn't work if an allocate function and > FD_{SET,CLR} functions > doesn't exist in the same place. This is the same limitation with > other string buffer > overflow checks. I inspected several other project codes. sudo ------------------------ check_input(int ttyfd, double *speed) { (snip) fdsr = ecalloc(howmany(ttyfd + 1, NFDBITS), sizeof(fd_mask)); for (;;) { FD_SET(ttyfd, fdsr); tv.tv_sec = 0; tv.tv_usec = 0; nready = select(ttyfd + 1, fdsr, NULL, NULL, paused ? NULL : &tv); ------------------------ In this case, __builtin_object_size() doesn't work too. However fixing is easy. We only need to annotate ecalloc as attribute((alloc_size(1,2))). rsyslog ------------------------- BEGINobjConstruct(nsdsel_ptcp) /* be sure to specify the object type also in END macro! */ pThis->maxfds = 0; #ifdef USE_UNLIMITED_SELECT pThis->pReadfds = calloc(1, glbl.GetFdSetSize()); pThis->pWritefds = calloc(1, glbl.GetFdSetSize()); #else FD_ZERO(&pThis->readfds); FD_ZERO(&pThis->writefds); #endif ENDobjConstruct(nsdsel_ptcp) ------------------------- In this case, __builtin_object_size() doesn't work too. because pThis->p{Read,Write}fds can be changed at runtime. compiler can't distingush the bitmap size. openssh ------------------------ static int timeout_connect(int sockfd, const struct sockaddr *serv_addr, socklen_t addrlen, int *timeoutp) { (snip) fdset = (fd_set *)xcalloc(howmany(sockfd + 1, NFDBITS), sizeof(fd_mask)); FD_SET(sockfd, fdset); ms_to_timeval(&tv, *timeoutp); for (;;) { rc = select(sockfd + 1, NULL, fdset, NULL, &tv); ------------------------ In this case, xcalloc also prevent to work __builtin_object_size(). It should be marded as attribute((alloc_size(1,2))). but, openssh ------------------------ void channel_prepare_select(fd_set **readsetp, fd_set **writesetp, int *maxfdp, u_int *nallocp, time_t *minwait_secs, int rekeying) { u_int n, sz, nfdset; n = MAX(*maxfdp, channel_max_fd); nfdset = howmany(n+1, NFDBITS); /* Explicitly test here, because xrealloc isn't always called */ if (nfdset && SIZE_T_MAX / nfdset < sizeof(fd_mask)) fatal("channel_prepare_select: max_fd (%d) is too large", n); sz = nfdset * sizeof(fd_mask); /* perhaps check sz < nalloc/2 and shrink? */ if (*readsetp == NULL || sz > *nallocp) { *readsetp = xrealloc(*readsetp, nfdset, sizeof(fd_mask)); *writesetp = xrealloc(*writesetp, nfdset, sizeof(fd_mask)); *nallocp = sz; } *maxfdp = n; memset(*readsetp, 0, sz); memset(*writesetp, 0, sz); if (!rekeying) channel_handler(channel_pre, *readsetp, *writesetp, minwait_secs); } ------------------------ This case is more harder. compiler can't know the size of readsetp and writesetp at compilering caller functions. librpcsecgss ------------------------ static int readtcp(ct, buf, len) register struct ct_data *ct; caddr_t buf; register int len; { (snip) if (ct->ct_sock+1 > FD_SETSIZE) { int bytes = howmany(ct->ct_sock+1, NFDBITS) * sizeof(fd_mask); fds = (fd_set *)malloc(bytes); if (fds == NULL) return (-1); memset(fds, 0, bytes); } else { fds = &readfds; FD_ZERO(fds); } gettimeofday(&start, NULL); delta = ct->ct_wait; while (TRUE) { /* XXX we know the other bits are still clear */ FD_SET(ct->ct_sock, fds); r = select(ct->ct_sock+1, fds, NULL, NULL, &delta); save_errno = errno; -------------------------- This is ok. compiler can understand the size of fds. bind9 ----------------------- static isc_result_t setup_watcher(isc_mem_t *mctx, isc__socketmgr_t *manager) { isc_result_t result; #if defined(USE_KQUEUE) || defined(USE_EPOLL) || defined(USE_DEVPOLL) char strbuf[ISC_STRERRORSIZE]; #endif (snip) #if ISC_SOCKET_MAXSOCKETS > FD_SETSIZE /* * Note: this code should also cover the case of MAXSOCKETS <= * FD_SETSIZE, but we separate the cases to avoid possible portability * issues regarding howmany() and the actual representation of fd_set. */ manager->fd_bufsize = howmany(manager->maxsocks, NFDBITS) * sizeof(fd_mask); #else manager->fd_bufsize = sizeof(fd_set); #endif --------------------------- This case also harder. FD_SET is called far different functions. Summary: alomost software only need to add alloc_size() annotation to xmalloc() or similar in almost case. but there are several exceptions. some software have a complicated fd size management and can't use __builtin_object_size(). but that's ok. In this case, the software correctly expand buffers by realloc() or similar, so there is no chance to happen buffer overflow.