Project

General

Profile

Actions

Bug #3725

open

Tunables shouldn't be used in MIN()/MAX()/nz() macros

Added by Marcel Telka over 10 years ago. Updated over 10 years ago.

Status:
In Progress
Priority:
Low
Assignee:
Category:
kernel
Start date:
2013-04-18
Due date:
% Done:

0%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage
Gerrit CR:
External Bug:

Description

Since the MIN macro evaluates both arguments twice, the value of count might be set to NFS_MAXDATA unexpectedly here in nfs_readdir:

3008    count = MIN(uiop->uio_iov->iov_len,
3009        nfs_shrinkreaddir ? 0x400 : NFS_MAXDATA);
#define    MIN(a, b)    ((a) < (b) ? (a) : (b))
Scenario:
  • assume nfs_shrinkreaddir is non-zero, so the second argument is 0x400
  • In a case the uiop->uio_iov->iov_len is greater (or equal) than 0x400, the second MIN's argument is going to be assigned to count
  • assume the nfs_shrinkreaddir in now changed to zero (it is a tunable so it could change anytime)
  • The second argument of MIN evaluates to NFS_MAXDATA unexpectedly and this value is assigned to count

The exactly same issue is in nfs3_readdir too.

Another occurence of the same issue in socktpi_plumbioctl():

6074        /*
6075         * Ceiling nmods at nstrpush to prevent someone from
6076         * maliciously consuming lots of kernel memory.
6077         */
6078        nmods = MIN(nmods, nstrpush);

In the ufs_vfsops.c file it is twice:

1197    } else {
1198        ufsvfsp->vfs_iotransz = MIN(maxphys, ufs_maxmaxphys);
1199    }
1200
1201    if (ufsvfsp->vfs_iotransz <= 0) {
1202        ufsvfsp->vfs_iotransz = MIN(maxphys, ufs_maxmaxphys);
1203    }

In ufs_vnops.c:

1432            smallfile1 = MAX(smallfile1, smallfile);

In arc.c:

3468        if (page_load > MAX(ptob(minfree), available_memory) / 4)

In mirror_resync.c:

2378        rs_copysize = MIN(md_resync_bufsz, md_max_xfer_bufsz);

In usr/src/uts/common/os/bio.c:

1136    bio_max_hwm = MIN(physmem / BIO_MAX_PERCENT,
1137        btop(vmem_size(heap_arena, VMEM_FREE)) / 4) * (PAGESIZE / 1024);
1153    bio_default_hwm = MIN(physmem / pct,
1154        btop(vmem_size(heap_arena, VMEM_FREE)) / 4) * (PAGESIZE / 1024);

In usr/src/uts/common/os/clock.c:

835            pgcnt_t avail =
836                MAX((spgcnt_t)(availrmem - swapfs_minfree), 0);

In dumpsubr.c:

477    new_size = dumpbuf_iosize(MAX(dumpbuf.iosize, maxphys));

In usr/src/uts/common/os/exec.c:

1208                deficit += MIN((pgcnt_t)(npages - availm),
1209                    lotsfree - deficit);

In usr/src/uts/common/os/sched.c:

343    needs = MIN(swapin_prp->p_swrss, lotsfree);
472                deficit += MIN(prp->p_swrss, lotsfree);

In stream.c:

3363                    val = MIN(strmsgsz, val);

In usr/src/uts/common/os/streamio.c:

579            rmax = MIN(strmsgsz, rmax);
3841                else    rmax = MIN(strmsgsz, rmax);
3922                else    rmax = MIN(strmsgsz, rmax);
4141                    rmax = MIN(strmsgsz, rmax);
4276                    else    rmax = MIN(strmsgsz, rmax);

In vm_meter.c:

128    deficit -= MIN(deficit,
129        MAX(deficit / 10, UsefulPagesPerIO * maxpgio / 2));

In vm_pageout.c:

434        slowscan = MIN(fastscan / 10, maxslowscan);
574            result = (slowstmp + faststmp) /
575                nz(lotsfree) / RATETOSCHEDPAGING;
579        pageout_ticks = min_pageout_ticks + (lotsfree - vavail) *
580            (max_pageout_ticks - min_pageout_ticks) / nz(lotsfree);
770    min_pageout_ticks = MAX(1,
771        ((hz * min_percent_cpu) / 100) / RATETOSCHEDPAGING);

In vm_seg.c:

1414        segpcache_hashsize_win = MAX(1024, physmem / pages_per_bucket);

In vm_swap.c

483        avail = MAX((spgcnt_t)(availrmem - swapfs_minfree), 0);
820        avail = MAX((spgcnt_t)(availrmem - swapfs_minfree), 0);
1410    if (((k_anoninfo.ani_max - k_anoninfo.ani_phys_resv) +
1411        MAX((spgcnt_t)(availrmem - swapfs_minfree), 0)) < pages) {

In usr/src/uts/common/disp/ts.c:

1606            epri += swapout_time - pp->p_swrss / nz(maxpgio)/2;
1677            epri = swapin_time -
1678                (rm_asrss(pp->p_as) / nz(maxpgio)/2) - (long)pri;

In usr/src/uts/common/disp/fss.c:

1859            epri += swapout_time - pp->p_swrss / nz(maxpgio)/2;
1912            epri = swapin_time -
1913                (rm_asrss(pp->p_as) / nz(maxpgio)/2) - (long)pri;

Similar case in tl.c:

966    tl_tidusz = strmsgsz != 0 ? (t_scalar_t)strmsgsz : TL_TIDUSZ;

Similar case in ptms_conf.c:

353        size_t user_max = (pt_max_pty == 0 ? ptms_ptymax : pt_max_pty);

Similar case in usr/src/uts/common/io/dump.c:

130            if (dumpsize_in_pages > (uint64_t)physmem)
131                dumpsize_in_pages = (uint64_t)physmem;

Related issues

Related to illumos gate - Bug #3720: Tunables needs volatile keywordIn ProgressMarcel Telka2013-04-16

Actions
Related to illumos gate - Bug #13567: maxpgio is too low for modern systemsNew

Actions
Actions #1

Updated by Marcel Telka over 10 years ago

  • Subject changed from nfs_readdir() counld send bigger e to nfs_readdir() counld send bigger requests than expected
Actions #2

Updated by Marcel Telka over 10 years ago

  • Subject changed from nfs_readdir() counld send bigger requests than expected to nfs_readdir() could send bigger requests than expected
Actions #3

Updated by Marcel Telka over 10 years ago

  • Subject changed from nfs_readdir() could send bigger requests than expected to nfs_readdir() or nfs3_readdir() could send bigger requests than expected
Actions #4

Updated by Marcel Telka over 10 years ago

  • Subject changed from nfs_readdir() or nfs3_readdir() could send bigger requests than expected to Tunables shouldn't be used in MIN() macro
  • Category changed from nfs - NFS server and client to kernel
  • Assignee set to Marcel Telka
Actions #5

Updated by Marcel Telka over 10 years ago

  • Subject changed from Tunables shouldn't be used in MIN() macro to Tunables shouldn't be used in MIN()/MAX() macros
Actions #6

Updated by Marcel Telka over 10 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Marcel Telka over 10 years ago

  • Subject changed from Tunables shouldn't be used in MIN()/MAX() macros to Tunables shouldn't be used in MIN()/MAX()/nz() macros
Actions #8

Updated by Joshua M. Clulow almost 3 years ago

  • Related to Bug #13567: maxpgio is too low for modern systems added
Actions

Also available in: Atom PDF