> Aside from the boilerplate, which it got mostly right as far as I can tell, the actual logic is hilariously wrong.
Please do tell, how is it hilariously wrong?
It seems to have written a factorial function just like it should, it implemented the logic to read the integer from /dev/factorial when a user-space program writes to it, and then it writes the result back to /dev/factorial, and it also returns the number of bytes written correctly.
Which was the entire point of the exercise. Also note that ChatGPT itself said it was just a sample and it might be incomplete.
I noticed it has a bug, because it reads `len` bytes instead of `sizeof(int)` bytes, but a programmer could have made the same mistake.
I would also use a fixed-size unsigned integer rather than simply `int` (as it can invoke UB on overflow). You can ask ChatGPT "what is wrong with this code?" and it can spit out the same arguments I'm making. In fact, it detected an infinite-loop bug on piece of complex code which I had just written and indeed, it had an infinite-loop bug.
Perhaps some additional logic to handle reading multiple integers and writing multiple answers could be written, but that would be a further iteration of the code, not the initial one that I would write.
If that is hilariously wrong, then I would also be hilariously wrong. And I'm not just some random web developer, I actually wrote Linux kernel code professionally for years (although, that was a very long time ago).
So, maybe it got some details wrong, but I could conceivably also get those details wrong until I tried to compile/run the code and see what was wrong.
> I would be more impressed if you used chat GPT to guide you through reverse engineering a piece of hardware and implementbing a driver for it.
Yes, I would be more impressed with that as well. Perhaps someone will do that sometime. Even if not with ChatGPT, perhaps with a future version of it or a similar model.
I'd expect the `copy_to_user` call to be inside `dev_read`, so that the userspace program can read the result with a further `read()` call, instead of mutating the buffer it gave to `write()` being mutated (that would probably not even work unless you used `write()` directly in your code instead of e.g. `fwrite()`).
Also as you noted, the logic related to handling `len` vs `sizeof(int)` is... curious.
While I find some ChatGPT examples impressive, this one isn't very enlightening. The block device structure and the factorial itself are fine, but those are likely in the training set as there are various tutorials online. But the combination of the factorial function into the block device is pretty awful (though I could imagine a novice doing it like this).
Both the OP and this driver kind of fit into this pattern, it looks like someone asked a novice programmer to do something using X and Y, so he read various few sample code and tutorials on X and Y and tries to put something together as best as it could, but the "plumbling" between X and Y (the part that wasn't in the tutorials) doesn't really work.
> I'd expect the `copy_to_user` call to be inside `dev_read`, so that the userspace program can read the result with a further `read()` call, instead of the buffer it gave to `write()` being mutated (that would probably not even work unless you used `write()` directly in your code instead of e.g. `fwrite()`).
Yes, you're probably right. It's been many years since I've written Linux kernel code and also, I've never implemented such a driver, so I missed that.
I asked ChatGPT: what is wrong with this Linux kernel code (and fed it the dev_write() function), and this is what it answered (on the first try):
> There are a few issues with this Linux kernel code:
> 1. The function dev_write() is declared as returning ssize_t, but the return value at the end of the function is sizeof(int). ssize_t and size_t are different types, so this could cause a type mismatch.
This is an interesting observation that you and I both missed.
> 2. The factorial() function is not defined in this code, so it is unclear what it does or where it is declared.
This is because I didn't feed it the factorial function nor the rest of the code: it was on a different chat session. One could guess what it does based on the name, but to be fair, it's not clear why such a function would be on the Linux kernel unless I gave it the full context.
> 3. The copy_from_user() and copy_to_user() functions are used to copy data between user space and kernel space. In this code, buffer is a pointer to user space, but input and result are local variables in kernel space. This means that copy_from_user() will copy data from user space into kernel space, but then copy_to_user() will copy data back from kernel space into the same buffer in user space, overwriting the original data. This is likely not the intended behavior.
I think this is basically the bug that you pointed out. At this point, it would be clear to me that the result would have to go back in dev_read() instead.
> 4. The len parameter is not used in this function, so it is unclear why it is included as an argument.
lol. Actually, it is being used, incorrectly.. and it wouldn't be used if we did the correction that I mentioned. So yes, this is somewhat hilariously wrong.
And it's clear why it has to be included as an argument, but to be fair, ChatGPT has no context whatsoever except knowing it's Linux kernel code (because I used a different chat session), so it's conceivable that it couldn't figure out why the function prototype has to be that way.
> Overall, this code is not well-written and would benefit from further clarification and revision.
It was you who wrote it, dumb-ass! :)
(Interestingly, ChatGPT can be compared to someone with Alzheimer in the sense that it can't remember conversations that happened in different chat sessions).
> > 1. The function dev_write() is declared as returning ssize_t, but the return value at the end of the function is sizeof(int). ssize_t and size_t are different types, so this could cause a type mismatch.
> This is an interesting observation that you and I both missed.
Hah. Call me when you find an architecture where ints use over half the addressable memory.
> Hah. Call me when you find an architecture where ints use over half the addressable memory.
I mean, I get your point if it's a joke :) But I think the AI was just pointing out that you'd get a compiler warning because of the type mismatch in signedness (is this even a word?).
It copies from user, and then copies to user, back into the const it got. I don't even know if this would compile but certainly nobody would expect the buffer they just wrote to a file to suddenly end up re-written with a "reply" when using a special file.
I found that logic very funny.
Edit: To elaborate, the actual code to do this properly would have to allocate some memory when the file was opened, this memory would hold the reply once a question was written. Then when the answer was read it would have to respond with that in there. Finally when the file is closed it would have to deallocate.
This code is quite far from accurate, the issue I have with the concept that even if this bot was 99% accurate, C is not a "try it and see" language. If you apply "try it and see" techniques to C you just get subtle undefined behaviour which you might not notice until someone uses it to remotely execute code on your machine. Really I am not so sure that even humans can be trusted to write C but at the very least I am well aware of all the instances where C is a minefield and can pay extra attention to those, if you simply look at the plethora of information on C out there to learn C and apply it to kernel development you won't automatically build these intuitions no matter how much code you read because an enormous amount of C out there is hideously wrong in many ways.
Final edit: Once you start looking at the details, it has managed to implement a bunch of buffer overflows to boot.
Please do tell, how is it hilariously wrong?
It seems to have written a factorial function just like it should, it implemented the logic to read the integer from /dev/factorial when a user-space program writes to it, and then it writes the result back to /dev/factorial, and it also returns the number of bytes written correctly.
Which was the entire point of the exercise. Also note that ChatGPT itself said it was just a sample and it might be incomplete.
I noticed it has a bug, because it reads `len` bytes instead of `sizeof(int)` bytes, but a programmer could have made the same mistake.
I would also use a fixed-size unsigned integer rather than simply `int` (as it can invoke UB on overflow). You can ask ChatGPT "what is wrong with this code?" and it can spit out the same arguments I'm making. In fact, it detected an infinite-loop bug on piece of complex code which I had just written and indeed, it had an infinite-loop bug.
Perhaps some additional logic to handle reading multiple integers and writing multiple answers could be written, but that would be a further iteration of the code, not the initial one that I would write.
If that is hilariously wrong, then I would also be hilariously wrong. And I'm not just some random web developer, I actually wrote Linux kernel code professionally for years (although, that was a very long time ago).
So, maybe it got some details wrong, but I could conceivably also get those details wrong until I tried to compile/run the code and see what was wrong.
> I would be more impressed if you used chat GPT to guide you through reverse engineering a piece of hardware and implementbing a driver for it.
Yes, I would be more impressed with that as well. Perhaps someone will do that sometime. Even if not with ChatGPT, perhaps with a future version of it or a similar model.