Clarification needed on use of put_user inside a loop

Hi All,

function put_user() is used to transfer small bytes of data (1-8 byte) from kernel space to user space and before transferring, it checks for the user's access over that memory area (in user space of-course) using function access_ok(). function __put_user() is used for same purpose but it skips checking permission part.

Hence when transferring data involves loops then checking permission (using access_ok()) once should be good to go then after we can simply transfer data using __put_user(), instead of using put_user() itself in loop.

I have found some codes in the driver which use put_user() in loop. Can we avoid the overhead of checking the same memory area( where put_user() writes) again n again using __put_user() in side loop and checking permission using access_ok before entering the loop?

Below is one of the codes I found.
File Name:sound/pci/hda/patch_hdmi.c

Code
-----------
for (i = 0; i < ARRAY_SIZE(channel_allocations); i++, cap++) { //line number 1928
int chs_bytes = chs * 4;
int type =

spec->ops.chmap_cea_alloc_validate_get_type(cap, chs);
                         unsigned int tlv_chmap[8];

                         if (type < 0)
                                 continue;
                         if (size < 8)
                                 return -ENOMEM;
                         if (put_user(type, dst) ||
                             put_user(chs_bytes, dst + 1))
                                 return -EFAULT;
                         dst += 2;
                         size -= 8;
                         count += 8;
                         if (size < chs_bytes)
                                 return -ENOMEM;
                         size -= chs_bytes;
                         count += chs_bytes;
                         spec->ops.cea_alloc_to_tlv_chmap(cap, 
tlv_chmap, chs);
                         if (copy_to_user(dst, tlv_chmap, chs_bytes))
                                 return -EFAULT;
                         dst += chs;
                 }

---------------------------
Please revert with comment on whether I am correct or not. If yes, I'll submit the patches for upgrading codes to skip the overhead of checking memory area for permission.

Regards,
Kumar Gaurav

Well, I can't tell you whether this is a good idea, but:

This looks correct and other code is doing this already.

However, put_user calls might_fault, but __put_user consumers I found (e.g. copy_siginfo_to_user) don't do that.

While it has only debugging purposes and would not change anything for those consumers, it seems to be a bug to not include it.

Thus I suggest adding access_ok variant which calls might_fault.

Mateusz Guzik

			  ^^^^^^^^^

Note that increment. It's *not* "the same memory area" next time around. Sure, you can check the whole range once before the loop and switch the stuff inside to __put_user()/__copy_to_user(), but it's not guaranteed to buy you any speedup.

BTW, you might be a bit confused about the work done by access_ok() - e.g. on an architectures with separate kernel and userland MMU contexts it might very well be a no-op (always return true). It's *not* checking if user has permissions of some sort.