17486 - Incorrect & operator precedence in _PR_UnixSendFile

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

In unix.c, _PR_UnixSendFile, we have:
================
    pagesize = PR_GetPageSize();
    /*
     * If the file is large, mmap and send the file in chunks so as
     * to not consume too much virtual address space
     */
    if ((sfd->file_offset == 0) ||
            (sfd->file_offset & (pagesize - 1) == 0)) {
        /*
         * case 1: page-aligned file offset
         */
================

Because the == operator has a higher precedence than
the & operator, the expression (sfd->file_offset & (pagesize - 1) == 0)
gets evaluated as follows:
===>    (sfd->file_offset & ((pagesize - 1) == 0))
===>    (sfd->file_offset & 0)    # assuming (pagesize - 1) is not 0
===>    0

The if statement reduces to:
    if (sfd->file_offset == 0) {

So we fail to detect a nonzero file offset that is page
aligned.  This results in the assertion failure
"addr_offset > 0" at mozilla/nsprpub/pr/src/md/unix.c (rev. 3.29),
line 3005.

Comment 1

[Wan-Teh Chang]

Attachment: A patch file for the socket.c test to cause the assertion failure

Comment 2

[Wan-Teh Chang]

Attachment: Proposed fix. Just added parentheses around the & operator.

Comment 3

[Wan-Teh Chang]

The fix is checked in.
/cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c, revision 3.30.

/m/src/ns/nspr20/pr/src/md/unix/unix.c, revision 2.68

Srinivas, could you look into how to add a new case (where
offset is a multiple of page size) to socket.c?  Thanks.

Comment 4

[Wan-Teh Chang]

This fix is checked into NSPR20_RELEASE_3_5_BRANCH (in the
internal /m/src repository), so it will be in the NSPR
3.5.1 patch release.
/m/src/ns/nspr20/pr/src/md/unix/unix.c, revision 2.65.2.1

Comment 5

[srinivas]

Added a new test case for PR_SendFile in socket.c, for a non-zero page-aligned
file offset.