Reading inside critical sections from ISR
You should use critical sections around any operations that could potentially be affected by the execution of an ISR (or which could affect an ISR) that aren't atomic.

Simply put, an atomic instruction is one which can be represented by a single assembly language instruction. You can't interrupt during the execution of an assembly instruction, so anything that only takes one instruction can't be interrupted until it's complete.

Examples would be moving a value from one 8-bit variable (assuming an 8-bit architecture) into a register, storing an 8-bit value into memory, etc. 

What instructions are likely to be atomic very much depends on the architecture that you are working on.  The only real way to be sure is to compile the code into assembly language and examine what is produced. Of course, that means learning the assembly language of your chosen architecture. 

However: if in doubt, use a critical section. Better to be safe than sorry. But keep those critical sections as short as possible to not disturb the interrupts more than is needed.

For example, instead of starting a critical section, reading some volatile value, doing some calculations with it, and writing it elsewhere, you should use the critical section just to read the volatile value into a temporary variable. After the critical section, you can then do your calculations and things with it, while the interrupt is then free to be called again.