commit a45b599ad808c ("scsi: sg: allocate with __GFP_ZERO in
sg_build_indirect()") changed the call to alloc_pages to always use
__GFP_ZERO.  Just above that, though, there was this:

       if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
               gfp_mask |= __GFP_ZERO;

And there's only one user of the gfp_mask.  Just or in the __GFP_ZERO
flag at the top of the function and be done with it.

Signed-off-by: Jeff Moyer <jmo...@redhat.com>

p.s. I'm not even sure why the original patch was committed -- it does
nothing for security.  I assume that the zeroing isn't done for
CAP_SYS_ADMIN / CAP_SYS_RAWIO because of performance.  (The history of
that zeroing being conditional goes way back to pre-git.)  Was there any
performance measurement done after the commit a45b599ad808c was applied?

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 53ae52dbff84..c926e07aabda 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1850,7 +1850,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, 
int buff_size)
        int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
        int sg_tablesize = sfp->parentdp->sg_tablesize;
        int blk_size = buff_size, order;
-       gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+       gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
        struct sg_device *sdp = sfp->parentdp;
 
        if (blk_size < 0)
@@ -1880,9 +1880,6 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, 
int buff_size)
        if (sdp->device->host->unchecked_isa_dma)
                gfp_mask |= GFP_DMA;
 
-       if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
-               gfp_mask |= __GFP_ZERO;
-
        order = get_order(num);
 retry:
        ret_sz = 1 << (PAGE_SHIFT + order);
@@ -1893,7 +1890,7 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, 
int buff_size)
                num = (rem_sz > scatter_elem_sz_prev) ?
                        scatter_elem_sz_prev : rem_sz;
 
-               schp->pages[k] = alloc_pages(gfp_mask | __GFP_ZERO, order);
+               schp->pages[k] = alloc_pages(gfp_mask, order);
                if (!schp->pages[k])
                        goto out;
 

Reply via email to