Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[PATCH] m68k: sys_ptrace cleanup

- create helper function singlestep_disable()
- move variable definitions to the top of the function
- use "out_eio" label as common error destination
- don't clear failure value for PTRACE_SETREGS/PTRACE_GETREGS

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

authored by

Roman Zippel and committed by
Linus Torvalds
69f447cf b3319f50

+65 -112
+65 -112
arch/m68k/kernel/ptrace.c
··· 103 103 } 104 104 105 105 /* 106 - * Called by kernel/ptrace.c when detaching.. 107 - * 108 106 * Make sure the single step bit is not set. 107 + */ 108 + static inline void singlestep_disable(struct task_struct *child) 109 + { 110 + unsigned long tmp = get_reg(child, PT_SR) & ~(TRACE_BITS << 16); 111 + put_reg(child, PT_SR, tmp); 112 + child->thread.work.delayed_trace = 0; 113 + } 114 + 115 + /* 116 + * Called by kernel/ptrace.c when detaching.. 109 117 */ 110 118 void ptrace_disable(struct task_struct *child) 111 119 { 112 - unsigned long tmp; 113 - /* make sure the single step bit is not set. */ 114 - tmp = get_reg(child, PT_SR) & ~(TRACE_BITS << 16); 115 - put_reg(child, PT_SR, tmp); 116 - child->thread.work.delayed_trace = 0; 120 + singlestep_disable(child); 117 121 child->thread.work.syscall_trace = 0; 118 122 } 119 123 120 124 asmlinkage int sys_ptrace(long request, long pid, long addr, long data) 121 125 { 122 126 struct task_struct *child; 123 - int ret; 127 + unsigned long tmp; 128 + int i, ret = 0; 124 129 125 130 lock_kernel(); 126 - ret = -EPERM; 127 131 if (request == PTRACE_TRACEME) { 128 132 /* are we already being traced? */ 129 - if (current->ptrace & PT_PTRACED) 133 + if (current->ptrace & PT_PTRACED) { 134 + ret = -EPERM; 130 135 goto out; 136 + } 131 137 /* set the ptrace bit in the process flags. */ 132 138 current->ptrace |= PT_PTRACED; 133 - ret = 0; 134 139 goto out; 135 140 } 136 - ret = -ESRCH; 137 141 read_lock(&tasklist_lock); 138 142 child = find_task_by_pid(pid); 139 143 if (child) 140 144 get_task_struct(child); 141 145 read_unlock(&tasklist_lock); 142 - if (!child) 146 + if (unlikely(!child)) { 147 + ret = -ESRCH; 143 148 goto out; 149 + } 144 150 145 - ret = -EPERM; 146 - if (pid == 1) /* you may not mess with init */ 151 + /* you may not mess with init */ 152 + if (unlikely(pid == 1)) { 153 + ret = -EPERM; 147 154 goto out_tsk; 155 + } 148 156 149 157 if (request == PTRACE_ATTACH) { 150 158 ret = ptrace_attach(child); ··· 160 152 } 161 153 162 154 ret = ptrace_check_attach(child, request == PTRACE_KILL); 163 - if (ret < 0) 155 + if (ret) 164 156 goto out_tsk; 165 157 166 158 switch (request) { 167 159 /* when I and D space are separate, these will need to be fixed. */ 168 160 case PTRACE_PEEKTEXT: /* read word at location addr. */ 169 - case PTRACE_PEEKDATA: { 170 - unsigned long tmp; 171 - int copied; 172 - 173 - copied = access_process_vm(child, addr, &tmp, sizeof(tmp), 0); 174 - ret = -EIO; 175 - if (copied != sizeof(tmp)) 176 - break; 161 + case PTRACE_PEEKDATA: 162 + i = access_process_vm(child, addr, &tmp, sizeof(tmp), 0); 163 + if (i != sizeof(tmp)) 164 + goto out_eio; 177 165 ret = put_user(tmp, (unsigned long *)data); 178 166 break; 179 - } 180 167 181 168 /* read the word at location addr in the USER area. */ 182 - case PTRACE_PEEKUSR: { 183 - unsigned long tmp; 169 + case PTRACE_PEEKUSR: 170 + if (addr & 3) 171 + goto out_eio; 172 + addr >>= 2; /* temporary hack. */ 184 173 185 - ret = -EIO; 186 - if ((addr & 3) || addr < 0 || 187 - addr > sizeof(struct user) - 3) 188 - break; 189 - 190 - tmp = 0; /* Default return condition */ 191 - addr = addr >> 2; /* temporary hack. */ 192 - ret = -EIO; 193 - if (addr < 19) { 174 + if (addr >= 0 && addr < 19) { 194 175 tmp = get_reg(child, addr); 195 176 if (addr == PT_SR) 196 177 tmp >>= 16; 197 178 } else if (addr >= 21 && addr < 49) { 198 179 tmp = child->thread.fp[addr - 21]; 199 - #ifdef CONFIG_M68KFPU_EMU 200 180 /* Convert internal fpu reg representation 201 181 * into long double format 202 182 */ 203 183 if (FPU_IS_EMU && (addr < 45) && !(addr % 3)) 204 184 tmp = ((tmp & 0xffff0000) << 15) | 205 185 ((tmp & 0x0000ffff) << 16); 206 - #endif 207 186 } else 208 187 break; 209 188 ret = put_user(tmp, (unsigned long *)data); 210 189 break; 211 - } 212 190 213 191 /* when I and D space are separate, this will have to be fixed. */ 214 192 case PTRACE_POKETEXT: /* write the word at location addr. */ 215 193 case PTRACE_POKEDATA: 216 - ret = 0; 217 - if (access_process_vm(child, addr, &data, sizeof(data), 1) == sizeof(data)) 218 - break; 219 - ret = -EIO; 194 + if (access_process_vm(child, addr, &data, sizeof(data), 1) != sizeof(data)) 195 + goto out_eio; 220 196 break; 221 197 222 198 case PTRACE_POKEUSR: /* write the word at location addr in the USER area */ 223 - ret = -EIO; 224 - if ((addr & 3) || addr < 0 || 225 - addr > sizeof(struct user) - 3) 226 - break; 227 - 228 - addr = addr >> 2; /* temporary hack. */ 199 + if (addr & 3) 200 + goto out_eio; 201 + addr >>= 2; /* temporary hack. */ 229 202 230 203 if (addr == PT_SR) { 231 204 data &= SR_MASK; 232 205 data <<= 16; 233 206 data |= get_reg(child, PT_SR) & ~(SR_MASK << 16); 234 - } 235 - if (addr < 19) { 207 + } else if (addr >= 0 && addr < 19) { 236 208 if (put_reg(child, addr, data)) 237 - break; 238 - ret = 0; 239 - break; 240 - } 241 - if (addr >= 21 && addr < 48) { 242 - #ifdef CONFIG_M68KFPU_EMU 209 + goto out_eio; 210 + } else if (addr >= 21 && addr < 48) { 243 211 /* Convert long double format 244 212 * into internal fpu reg representation 245 213 */ ··· 224 240 data = (data & 0xffff0000) | 225 241 ((data & 0x0000ffff) >> 1); 226 242 } 227 - #endif 228 243 child->thread.fp[addr - 21] = data; 229 - ret = 0; 230 - } 244 + } else 245 + goto out_eio; 231 246 break; 232 247 233 248 case PTRACE_SYSCALL: /* continue and stop at next (return from) syscall */ 234 - case PTRACE_CONT: { /* restart after signal. */ 235 - long tmp; 236 - 237 - ret = -EIO; 249 + case PTRACE_CONT: /* restart after signal. */ 238 250 if (!valid_signal(data)) 239 - break; 240 - if (request == PTRACE_SYSCALL) { 251 + goto out_eio; 252 + 253 + if (request == PTRACE_SYSCALL) 241 254 child->thread.work.syscall_trace = ~0; 242 - } else { 255 + else 243 256 child->thread.work.syscall_trace = 0; 244 - } 245 257 child->exit_code = data; 246 - /* make sure the single step bit is not set. */ 247 - tmp = get_reg(child, PT_SR) & ~(TRACE_BITS << 16); 248 - put_reg(child, PT_SR, tmp); 249 - child->thread.work.delayed_trace = 0; 258 + singlestep_disable(child); 250 259 wake_up_process(child); 251 - ret = 0; 252 260 break; 253 - } 254 261 255 262 /* 256 263 * make the child exit. Best I can do is send it a sigkill. 257 264 * perhaps it should be put in the status that it wants to 258 265 * exit. 259 266 */ 260 - case PTRACE_KILL: { 261 - long tmp; 262 - 263 - ret = 0; 267 + case PTRACE_KILL: 264 268 if (child->exit_state == EXIT_ZOMBIE) /* already dead */ 265 269 break; 266 270 child->exit_code = SIGKILL; 267 - /* make sure the single step bit is not set. */ 268 - tmp = get_reg(child, PT_SR) & ~(TRACE_BITS << 16); 269 - put_reg(child, PT_SR, tmp); 270 - child->thread.work.delayed_trace = 0; 271 + singlestep_disable(child); 271 272 wake_up_process(child); 272 273 break; 273 - } 274 274 275 - case PTRACE_SINGLESTEP: { /* set the trap flag. */ 276 - long tmp; 277 - 278 - ret = -EIO; 275 + case PTRACE_SINGLESTEP: /* set the trap flag. */ 279 276 if (!valid_signal(data)) 280 - break; 277 + goto out_eio; 278 + 281 279 child->thread.work.syscall_trace = 0; 282 280 tmp = get_reg(child, PT_SR) | (TRACE_BITS << 16); 283 281 put_reg(child, PT_SR, tmp); ··· 268 302 child->exit_code = data; 269 303 /* give it a chance to run. */ 270 304 wake_up_process(child); 271 - ret = 0; 272 305 break; 273 - } 274 306 275 307 case PTRACE_DETACH: /* detach a process that was attached. */ 276 308 ret = ptrace_detach(child, data); 277 309 break; 278 310 279 - case PTRACE_GETREGS: { /* Get all gp regs from the child. */ 280 - int i; 281 - unsigned long tmp; 311 + case PTRACE_GETREGS: /* Get all gp regs from the child. */ 282 312 for (i = 0; i < 19; i++) { 283 313 tmp = get_reg(child, i); 284 314 if (i == PT_SR) 285 315 tmp >>= 16; 286 - if (put_user(tmp, (unsigned long *)data)) { 287 - ret = -EFAULT; 316 + ret = put_user(tmp, (unsigned long *)data); 317 + if (ret) 288 318 break; 289 - } 290 319 data += sizeof(long); 291 320 } 292 - ret = 0; 293 321 break; 294 - } 295 322 296 - case PTRACE_SETREGS: { /* Set all gp regs in the child. */ 297 - int i; 298 - unsigned long tmp; 323 + case PTRACE_SETREGS: /* Set all gp regs in the child. */ 299 324 for (i = 0; i < 19; i++) { 300 - if (get_user(tmp, (unsigned long *)data)) { 301 - ret = -EFAULT; 325 + ret = get_user(tmp, (unsigned long *)data); 326 + if (ret) 302 327 break; 303 - } 304 328 if (i == PT_SR) { 305 329 tmp &= SR_MASK; 306 330 tmp <<= 16; ··· 299 343 put_reg(child, i, tmp); 300 344 data += sizeof(long); 301 345 } 302 - ret = 0; 303 346 break; 304 - } 305 347 306 - case PTRACE_GETFPREGS: { /* Get the child FPU state. */ 307 - ret = 0; 348 + case PTRACE_GETFPREGS: /* Get the child FPU state. */ 308 349 if (copy_to_user((void *)data, &child->thread.fp, 309 350 sizeof(struct user_m68kfp_struct))) 310 351 ret = -EFAULT; 311 352 break; 312 - } 313 353 314 - case PTRACE_SETFPREGS: { /* Set the child FPU state. */ 315 - ret = 0; 354 + case PTRACE_SETFPREGS: /* Set the child FPU state. */ 316 355 if (copy_from_user(&child->thread.fp, (void *)data, 317 356 sizeof(struct user_m68kfp_struct))) 318 357 ret = -EFAULT; 319 358 break; 320 - } 321 359 322 360 default: 323 361 ret = ptrace_request(child, request, addr, data); ··· 322 372 out: 323 373 unlock_kernel(); 324 374 return ret; 375 + out_eio: 376 + ret = -EIO; 377 + goto out_tsk; 325 378 } 326 379 327 380 asmlinkage void syscall_trace(void)